From 918043e64e0b214579ecf83e67b1ee1664171e69 Mon Sep 17 00:00:00 2001 From: LebToki <957618+LebToki@users.noreply.github.com> Date: Sat, 21 Mar 2026 10:43:37 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20Path=20Traversal=20Bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a path traversal bypass vulnerability in `public/api/files.php`, `public/api/terminal.php`, `public/api/projects.php`, and `src/Utils/Security.php` where validating if a user-supplied path is within an allowed directory using `strpos($realPath, $allowedReal) === 0` allowed access to sibling directories (e.g., an allowed path `/var/www` matched the malicious path `/var/www_backup`). Fixed by ensuring exact path matches or appending a trailing directory separator (`/`) to the allowed path prefix. --- .jules/sentinel.md | 5 +++++ public/api/files.php | 16 +++++++++++++--- public/api/projects.php | 13 +++++++++++-- public/api/terminal.php | 16 +++++++++++++--- src/Utils/Security.php | 13 +++++++++++-- 5 files changed, 53 insertions(+), 10 deletions(-) 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; + } } }