Merge branch '4.1' into 'main'

Author: thorsten

phpmyfaq/admin/assets/src/content/editor.ts

@@ -266,6 +266,9 @@ export const renderEditor = () => {
       ajax: {
         url: './api/media-browser',
         contentType: 'application/json; charset=UTF-8',
+        data: {
+          csrfToken: (document.getElementById('pmf-csrf-token') as HTMLInputElement).value,
+        },
       },
       createNewFolder: false,
       deleteFolder: false,

phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/MediaBrowserController.php

@@ -24,6 +24,7 @@
 use phpMyFAQ\Core\Exception;
 use phpMyFAQ\Enums\PermissionType;
 use phpMyFAQ\Filter;
+use phpMyFAQ\Session\Token;
 use phpMyFAQ\Translation;
 use phpMyFAQ\Utils;
 use RecursiveDirectoryIterator;
@@ -58,11 +59,23 @@ public function index(Request $request): JsonResponse|Response
         $action = Filter::filterVar($data->action, FILTER_SANITIZE_SPECIAL_CHARS);
 
         if ($action === 'fileRemove') {
-            $file = Filter::filterVar($data->name, FILTER_SANITIZE_SPECIAL_CHARS);
-            $file = PMF_CONTENT_DIR . '/user/images/' . $file;
+            if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken(
+                'media-browser',
+                $data->csrfToken,
+            )) {
+                return $this->json(['error' => Translation::get(key: 'msgNoPermission')], Response::HTTP_UNAUTHORIZED);
+            }
+
+            $file = basename(Filter::filterVar($data->name, FILTER_SANITIZE_SPECIAL_CHARS));
+            $allowedDir = realpath(PMF_CONTENT_DIR . '/user/images');
+            $targetPath = realpath(PMF_CONTENT_DIR . '/user/images/' . $file);
+
+            if ($targetPath === false || !str_starts_with($targetPath, $allowedDir . DIRECTORY_SEPARATOR)) {
+                return $this->json(['error' => 'Invalid file path'], Response::HTTP_BAD_REQUEST);
+            }
 
-            if (file_exists($file)) {
-                unlink($file);
+            if (file_exists($targetPath)) {
+                unlink($targetPath);
             }
 
             $response = [

phpmyfaq/src/phpMyFAQ/Filter.php

@@ -171,10 +171,16 @@ public static function removeAttributes(string $html = ''): string
         // remove broken stuff
         $html = str_replace(search: '
', replace: '', subject: $html);
 
-        preg_match_all(pattern: '/[a-z]+=".+"/iU', subject: $html, matches: $attributes);
+        // Match attributes with double quotes, single quotes, or no quotes
+        preg_match_all(
+            pattern: '/[a-z]+\s*=\s*(?:"[^"]*"|\'[^\']*\'|[^\s>]+)/iU',
+            subject: $html,
+            matches: $attributes,
+        );
 
         foreach ($attributes[0] as $attribute) {
             $attributeName = stristr($attribute, needle: '=', before_needle: true);
+            $attributeName = trim($attributeName);
             if (!self::isAttribute($attributeName)) {
                 continue;
             }

tests/phpMyFAQ/FilterTest.php

@@ -297,6 +297,40 @@ public function testComplexHtmlFiltering(): void
         $this->assertStringNotContainsString('onmouseover', $result);
     }
 
+    public function testRemoveAttributesWithUnquotedValues(): void
+    {
+        $html = '<img src=x onerror=alert(1)>';
+        $result = Filter::removeAttributes($html);
+
+        $this->assertStringNotContainsString('onerror', $result);
+    }
+
+    public function testRemoveAttributesWithSingleQuotedValues(): void
+    {
+        $html = "<img src='x' onerror='alert(1)'>";
+        $result = Filter::removeAttributes($html);
+
+        $this->assertStringNotContainsString('onerror', $result);
+    }
+
+    public function testRemoveAttributesWithSvgOnload(): void
+    {
+        $html = '<svg onload=alert(1)>';
+        $result = Filter::removeAttributes($html);
+
+        $this->assertStringNotContainsString('onload', $result);
+    }
+
+    public function testRemoveAttributesWithMixedQuoteStyles(): void
+    {
+        $html = '<div class="safe" onclick=alert(1) style=\'color:red\' onmouseover="steal()">';
+        $result = Filter::removeAttributes($html);
+
+        $this->assertStringContainsString('class="safe"', $result);
+        $this->assertStringNotContainsString('onclick', $result);
+        $this->assertStringNotContainsString('onmouseover', $result);
+    }
+
     protected function tearDown(): void
     {
         unset($_GET['test_var'], $_GET['special_test'], $_POST['name'], $_POST['email']);