diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 8f4c12e..6413106 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -13,3 +13,8 @@ **Learning:** When dealing with structured text files where line breaks determine the structure (like `.env`, `ini`, or CSV files), user input MUST be explicitly stripped of all newline characters (`\r` and `\n`) before interpolation. Relying only on `trim()` is insufficient as it only removes whitespace at the boundaries, not within the payload. **Prevention:** Sanitize all user input destined for line-based configuration files by explicitly stripping `\r` and `\n` characters (e.g., using `str_replace(["\r", "\n"], '', $value)` in PHP) to ensure the input remains strictly on a single line. + +## 2024-05-25 - Path Validation Bypass (Directory Traversal) +**Vulnerability:** In `validatePath` and `isValidProjectPath` across multiple API files (`files.php`, `terminal.php`, `projects.php`, `src/Utils/Security.php`), the code used `strpos($realPath, $allowedReal) === 0` to check if a user-supplied path was within an allowed directory. +**Learning:** This is a classic path traversal bypass in PHP. Because it only checks for a string prefix, an allowed path like `/var/www` will successfully match a malicious sibling directory like `/var/www_backup` or `/var/www-secret`. +**Prevention:** Always append a trailing directory separator (`/`) to the allowed path prefix before checking with `strpos`, or perform an exact match if the paths are identical. For example: `strpos($realPath, rtrim($allowedReal, '/') . '/') === 0`. diff --git a/public/api/files.php b/public/api/files.php index 5f1d876..915610c 100644 --- a/public/api/files.php +++ b/public/api/files.php @@ -120,9 +120,19 @@ function validatePath($path, $allowedPaths) { $isValid = false; foreach ($allowedPaths as $allowed) { $allowedReal = realpath($allowed); - if ($allowedReal && strpos($realPath, $allowedReal) === 0) { - $isValid = true; - break; + if ($allowedReal) { + if ($realPath === $allowedReal) { + $isValid = true; + break; + } + + $normalizedReal = str_replace('\\', '/', $realPath); + $normalizedAllowed = rtrim(str_replace('\\', '/', $allowedReal), '/') . '/'; + + if (strpos($normalizedReal, $normalizedAllowed) === 0) { + $isValid = true; + break; + } } } diff --git a/public/api/projects.php b/public/api/projects.php index 5f915e4..3eac660 100644 --- a/public/api/projects.php +++ b/public/api/projects.php @@ -1181,8 +1181,17 @@ function isValidProjectPath($path, $workspaces) { foreach ($workspaces as $ws) { $wsPath = realpath($ws['path']); - if ($wsPath && strpos($realPath, $wsPath) === 0) { - return true; + if ($wsPath) { + if ($realPath === $wsPath) { + return true; + } + + $normalizedReal = str_replace('\\', '/', $realPath); + $normalizedAllowed = rtrim(str_replace('\\', '/', $wsPath), '/') . '/'; + + if (strpos($normalizedReal, $normalizedAllowed) === 0) { + return true; + } } } diff --git a/public/api/terminal.php b/public/api/terminal.php index 155b6a3..a8cdaf3 100644 --- a/public/api/terminal.php +++ b/public/api/terminal.php @@ -124,9 +124,19 @@ function validatePath($path, $allowedPaths) { $isValid = false; foreach ($allowedPaths as $allowed) { $allowedReal = realpath($allowed); - if ($allowedReal && strpos($realPath, $allowedReal) === 0) { - $isValid = true; - break; + if ($allowedReal) { + if ($realPath === $allowedReal) { + $isValid = true; + break; + } + + $normalizedReal = str_replace('\\', '/', $realPath); + $normalizedAllowed = rtrim(str_replace('\\', '/', $allowedReal), '/') . '/'; + + if (strpos($normalizedReal, $normalizedAllowed) === 0) { + $isValid = true; + break; + } } } diff --git a/src/Utils/Security.php b/src/Utils/Security.php index fc334dd..833cc01 100644 --- a/src/Utils/Security.php +++ b/src/Utils/Security.php @@ -45,8 +45,17 @@ public static function validateFilePath(string $path, array $allowedPaths): ?str foreach ($allowedPaths as $allowedPath) { $realAllowedPath = realpath($allowedPath); - if ($realAllowedPath && strpos($realPath, $realAllowedPath) === 0) { - return $realPath; + if ($realAllowedPath) { + if ($realPath === $realAllowedPath) { + return $realPath; + } + + $normalizedReal = str_replace('\\', '/', $realPath); + $normalizedAllowed = rtrim(str_replace('\\', '/', $realAllowedPath), '/') . '/'; + + if (strpos($normalizedReal, $normalizedAllowed) === 0) { + return $realPath; + } } }