From 16f3a5af666dc0a862291a0ea19a54e7ab92f304 Mon Sep 17 00:00:00 2001
From: Artur Kyryliuk <mail@artur.work>
Date: Fri, 11 Sep 2026 17:55:11 +0200
Subject: [PATCH] fix(manager): harden file manager tokens, media browser CSRF
 and alert output

- makeToken() built the file manager's one-shot token from uniqid(), which is
  clock-derived; it now uses random_bytes() and checkToken() compares with
  hash_equals().
- The media browser (mcpuk) had no CSRF check at all and relied on SameSite.
  browse.php now requires the session _token for every act except the page
  itself and its thumbnails; the scripts append it through baseGetData(), the
  single place every request URL is built.
- webAlertAndQuit() wrote its message into the alert page verbatim, so any
  caller interpolating request data was reflected XSS. The message is shown as
  plain text (it is read via textContent and removed), so it is escaped now.
- ?stay= and ?tab= are only ever small integers: cast them before they reach
  Location headers and a script block.
- Default upload_files/upload_images no longer list svg, htaccess or flash
  extensions; svg is scriptable and served same-origin, which gave the
  assets_files right a stored XSS. Only new installs pick the defaults up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0136CYHW4NXMDFA8gfuNCx7R
---
 core/factory/settings.php                     |  5 +-
 core/functions/actions/files.php              |  4 +-
 core/src/Controllers/Users/EditOrNewUser.php  |  2 +-
 core/src/Core.php                             |  4 +-
 .../FileManagerAndAlertHardeningTest.php      | 80 +++++++++++++++++++
 manager/actions/bkmanager.static.php          |  2 +-
 manager/media/browser/mcpuk/browse.php        | 12 +++
 .../media/browser/mcpuk/js/browser/misc.js    |  2 +
 .../browser/mcpuk/tpl/tpl_javascript.php      |  1 +
 manager/processors/save_content.processor.php |  4 +-
 .../processors/save_htmlsnippet.processor.php |  4 +-
 manager/processors/save_module.processor.php  |  4 +-
 manager/processors/save_plugin.processor.php  |  4 +-
 manager/processors/save_snippet.processor.php |  4 +-
 .../processors/save_template.processor.php    |  4 +-
 .../processors/save_tmplvars.processor.php    |  4 +-
 16 files changed, 118 insertions(+), 22 deletions(-)
 create mode 100644 core/tests/Unit/Security/FileManagerAndAlertHardeningTest.php

diff --git a/core/factory/settings.php b/core/factory/settings.php
index 07ca7228f9..5c15dd76dc 100644
--- a/core/factory/settings.php
+++ b/core/factory/settings.php
@@ -54,8 +54,9 @@
     'use_editor' => 1,
     'editor_css_path' => '',
     'filemanager_path' => '[(base_path)]',
-    'upload_files' => 'bmp,ico,gif,jpeg,jpg,png,psd,tif,tiff,fla,flv,swf,aac,au,avi,css,cache,doc,docx,gz,gzip,htaccess,htm,html,js,mp3,mp4,mpeg,mpg,ods,odp,odt,pdf,ppt,pptx,rar,tar,tgz,txt,wav,wmv,xls,xlsx,xml,z,zip,JPG,JPEG,PNG,GIF,svg,tpl,webp,avif',
-    'upload_images' => 'bmp,ico,gif,jpeg,jpg,png,psd,tif,tiff,svg,webp,avif',
+    // no svg (scriptable, same-origin), htaccess or flash; defaults only reach new installs
+    'upload_files' => 'bmp,ico,gif,jpeg,jpg,png,psd,tif,tiff,aac,au,avi,css,cache,doc,docx,gz,gzip,htm,html,js,mp3,mp4,mpeg,mpg,ods,odp,odt,pdf,ppt,pptx,rar,tar,tgz,txt,wav,wmv,xls,xlsx,xml,z,zip,JPG,JPEG,PNG,GIF,tpl,webp,avif',
+    'upload_images' => 'bmp,ico,gif,jpeg,jpg,png,psd,tif,tiff,webp,avif',
     'upload_media' => 'au,avi,mp3,mp4,mpeg,mpg,wav,wmv',
     'upload_maxsize' => '5000000',
     'new_file_permissions' => '0644',
diff --git a/core/functions/actions/files.php b/core/functions/actions/files.php
index 3add29b8a5..d87bfcb12a 100644
--- a/core/functions/actions/files.php
+++ b/core/functions/actions/files.php
@@ -813,7 +813,7 @@ function checkToken()
             $token = false;
         }
 
-        if (isset($_SESSION['token']) && !empty($_SESSION['token']) && $_SESSION['token'] === $token) {
+        if (is_string($token) && isset($_SESSION['token']) && !empty($_SESSION['token']) && hash_equals($_SESSION['token'], $token)) {
             $rs = true;
         } else {
             $rs = false;
@@ -830,7 +830,7 @@ function checkToken()
      */
     function makeToken()
     {
-        $newToken = uniqid('', true);
+        $newToken = bin2hex(random_bytes(16)); // uniqid() is clock-derived, not random
         $_SESSION['token'] = $newToken;
 
         return $newToken;
diff --git a/core/src/Controllers/Users/EditOrNewUser.php b/core/src/Controllers/Users/EditOrNewUser.php
index 38f6950ef1..0f50a961c4 100644
--- a/core/src/Controllers/Users/EditOrNewUser.php
+++ b/core/src/Controllers/Users/EditOrNewUser.php
@@ -157,7 +157,7 @@ public function process(): bool
 
         if ($userData['stay'] != '') {
             $a = ($userData['stay'] == '2') ? "88&id={$user->getKey()}" : "87";
-            $this->parameters['url'] = "index.php?a={$a}&r=2&stay=" . $userData['stay'];
+            $this->parameters['url'] = "index.php?a={$a}&r=2&stay=" . (int)$userData['stay'];
         } else {
             $this->parameters['url'] = "index.php?a=88&id={$user->getKey()}";
         }
diff --git a/core/src/Core.php b/core/src/Core.php
index f7dd4269fa..c2e1388a98 100644
--- a/core/src/Core.php
+++ b/core/src/Core.php
@@ -3595,7 +3595,7 @@ public function getChildIds($id, $depth = 10, $children = [])
     /**
      * Displays a javascript alert message in the web browser and quit
      *
-     * @param string $msg Message to show
+     * @param string $msg Message to show, as plain text
      * @param string $url URL to redirect to
      */
     public function webAlertAndQuit($msg, $url = '')
@@ -3648,7 +3648,7 @@ function __alertQuit() {
                 </script>
             </head>
             <body>
-                <p>" . $msg . '</p>
+                <p>" . htmlspecialchars((string)$msg, ENT_QUOTES, $manager_charset) . '</p>
             </body>
         </html>';
         exit;
diff --git a/core/tests/Unit/Security/FileManagerAndAlertHardeningTest.php b/core/tests/Unit/Security/FileManagerAndAlertHardeningTest.php
new file mode 100644
index 0000000000..97d00d6cf2
--- /dev/null
+++ b/core/tests/Unit/Security/FileManagerAndAlertHardeningTest.php
@@ -0,0 +1,80 @@
+<?php
+
+/*
+|--------------------------------------------------------------------------
+| File manager tokens, media browser CSRF, alert escaping
+|--------------------------------------------------------------------------
+|
+| The file manager guards its destructive GET actions with a one-shot session token, which was
+| built from uniqid() - a clock value, not a random one. The media browser (mcpuk) had no CSRF
+| check at all and relied on SameSite alone. webAlertAndQuit() wrote its message into the page
+| verbatim, so any caller interpolating request data into an alert was a reflected XSS. The
+| ?stay= and ?tab= parameters were concatenated into Location headers and a script block; they
+| are only ever small integers.
+|
+*/
+
+function repoSource(string $relative): string
+{
+    return (string)file_get_contents(dirname(__DIR__, 4) . '/' . $relative);
+}
+
+it('builds the file manager token from random bytes and compares it in constant time', function () {
+    $source = repoSource('core/functions/actions/files.php');
+
+    expect($source)
+        ->toContain('$newToken = bin2hex(random_bytes(16));')
+        ->toContain("hash_equals(\$_SESSION['token'], \$token)")
+        ->and($source)->not->toContain("uniqid('', true)");
+});
+
+it('requires the session CSRF token for every media browser act except the page and its thumbnails', function () {
+    $entry = repoSource('manager/media/browser/mcpuk/browse.php');
+
+    expect($entry)
+        ->toContain("in_array(\$act, ['browser', 'thumb'], true)")
+        ->toContain('hash_equals(csrf_token(), $token)');
+
+    // the check must run before the browser is instantiated
+    expect(strpos($entry, 'hash_equals(csrf_token()'))->toBeLessThan(strpos($entry, 'new browser('));
+
+    // ... and the scripts have to send it: every request URL is built by baseGetData()
+    expect(repoSource('manager/media/browser/mcpuk/tpl/tpl_javascript.php'))
+        ->toContain('browser.csrfToken = "<?php echo text::jsValue(csrf_token()) ?>";');
+    expect(repoSource('manager/media/browser/mcpuk/js/browser/misc.js'))
+        ->toContain('data += "&_token=" + encodeURIComponent(this.csrfToken);');
+});
+
+it('escapes the message webAlertAndQuit writes into the alert page', function () {
+    $source = repoSource('core/src/Core.php');
+
+    expect($source)
+        ->toContain("<p>\" . htmlspecialchars((string)\$msg, ENT_QUOTES, \$manager_charset) . '</p>")
+        ->and($source)->not->toContain("<p>\" . \$msg . '</p>");
+});
+
+it('casts the stay parameter before echoing it into a redirect', function (string $file) {
+    $source = repoSource($file);
+
+    expect($source)->not->toMatch('/stay=[\'"] \. \$_POST\[\'stay\'\]/');
+})->with(array_map(
+    static fn (string $path) => 'manager/processors/' . basename($path),
+    glob(dirname(__DIR__, 4) . '/manager/processors/save_*.processor.php')
+));
+
+it('casts the backup manager tab index before writing it into a script block', function () {
+    expect(repoSource('manager/actions/bkmanager.static.php'))
+        ->toContain("tpDBM.setSelectedIndex( ' . (int)\$_GET['tab'] . ' );");
+});
+
+it('ships upload defaults without scriptable or server-config extensions', function () {
+    // the factory file needs a booted manager, so read the two lines instead of requiring it
+    $source = repoSource('core/factory/settings.php');
+
+    foreach (['upload_files', 'upload_images'] as $key) {
+        expect(preg_match("/'$key' => '([^']*)'/", $source, $m))->toBe(1);
+        $list = explode(',', $m[1]);
+        expect(array_intersect($list, ['svg', 'htaccess', 'swf', 'fla', 'flv', 'php', 'phtml', 'phar']))
+            ->toBe([], "$key allows a dangerous extension");
+    }
+});
diff --git a/manager/actions/bkmanager.static.php b/manager/actions/bkmanager.static.php
index c76b81ba5c..8ad2eb50f3 100755
--- a/manager/actions/bkmanager.static.php
+++ b/manager/actions/bkmanager.static.php
@@ -603,7 +603,7 @@ class="<?= $_style['icon_save'] ?>"></i> <?= $_lang["bkmgr_snapshot_submit"] ?>
 
 $tab = get_by_key($_GET, 'tab', false);
 if (is_numeric($tab)) {
-    echo '<script type="text/javascript">tpDBM.setSelectedIndex( ' . $_GET['tab'] . ' );</script>';
+    echo '<script type="text/javascript">tpDBM.setSelectedIndex( ' . (int)$_GET['tab'] . ' );</script>';
 }
 
 include_once EVO_MANAGER_PATH . "includes/footer.inc.php"; // send footer
diff --git a/manager/media/browser/mcpuk/browse.php b/manager/media/browser/mcpuk/browse.php
index 163159b6eb..588cc7c612 100755
--- a/manager/media/browser/mcpuk/browse.php
+++ b/manager/media/browser/mcpuk/browse.php
@@ -23,5 +23,17 @@ function returnNoPermissionsMessage($role) {
 if( $_GET['type'] == 'images' && !EvolutionCMS()->hasPermission('file_manager') && !EvolutionCMS()->hasPermission('assets_images')) returnNoPermissionsMessage('assets_images');
 if( $_GET['type'] == 'files'  && !EvolutionCMS()->hasPermission('file_manager') && !EvolutionCMS()->hasPermission('assets_files'))  returnNoPermissionsMessage('assets_files');
 
+// Only the page itself and the thumbnails it embeds are fetched without a token; every other
+// act, including reads, is scripted through browser.baseGetData() and carries the session one.
+$act = isset($_GET['act']) ? $_GET['act'] : 'browser';
+if (!in_array($act, ['browser', 'thumb'], true)) {
+    $token = isset($_REQUEST['_token']) && is_string($_REQUEST['_token']) ? $_REQUEST['_token'] : '';
+    if ($token === '' || !hash_equals(csrf_token(), $token)) {
+        header('HTTP/1.1 403 Forbidden');
+        header('Content-Type: text/plain; charset=utf-8');
+        die(json_encode(['error' => 'Invalid CSRF token.']));
+    }
+}
+
 $browser = new browser($modx);
 $browser->action();
diff --git a/manager/media/browser/mcpuk/js/browser/misc.js b/manager/media/browser/mcpuk/js/browser/misc.js
index 11d5645d70..3e899d0c06 100755
--- a/manager/media/browser/mcpuk/js/browser/misc.js
+++ b/manager/media/browser/mcpuk/js/browser/misc.js
@@ -339,6 +339,8 @@ browser.baseGetData = function(act) {
         data += "&act=" + act;
     if (this.cms)
         data += "&cms=" + this.cms;
+    if (this.csrfToken)
+        data += "&_token=" + encodeURIComponent(this.csrfToken);
     return data;
 };
 
diff --git a/manager/media/browser/mcpuk/tpl/tpl_javascript.php b/manager/media/browser/mcpuk/tpl/tpl_javascript.php
index 47fb459624..a532d3f192 100755
--- a/manager/media/browser/mcpuk/tpl/tpl_javascript.php
+++ b/manager/media/browser/mcpuk/tpl/tpl_javascript.php
@@ -43,6 +43,7 @@
 browser.opener.TinyMCE4 = "<?= text::jsValue($this->get['field']) ?>";
 <?php ENDIF ?>
 browser.cms = "<?php echo text::jsValue($this->cms) ?>";
+browser.csrfToken = "<?php echo text::jsValue(csrf_token()) ?>";
 _.kuki.domain = "<?php echo text::jsValue($this->config['cookieDomain']) ?>";
 _.kuki.path = "<?php echo text::jsValue($this->config['cookiePath']) ?>";
 _.kuki.prefix = "<?php echo text::jsValue($this->config['cookiePrefix']) ?>";
diff --git a/manager/processors/save_content.processor.php b/manager/processors/save_content.processor.php
index e5b2459247..226e1ec937 100755
--- a/manager/processors/save_content.processor.php
+++ b/manager/processors/save_content.processor.php
@@ -476,7 +476,7 @@
             } else {
                 $a = ($_POST['stay'] == '2') ? "27&id=$key" : "4&pid=$parentId";
             }
-            $redirectUrl = "index.php?a=" . $a . "&r=1&stay=" . $_POST['stay'];
+            $redirectUrl = "index.php?a=" . $a . "&r=1&stay=" . (int)$_POST['stay'];
         } else {
             $redirectUrl = "index.php?a=3&id=$key&r=1";
         }
@@ -689,7 +689,7 @@
                     // document
                     $a = ($_POST['stay'] == '2') ? "27&id=$id" : "4&pid=$parentId";
                 }
-                $redirectUrl = "index.php?a=" . $a . "&r=1&stay=" . $_POST['stay'] . $add_path;
+                $redirectUrl = "index.php?a=" . $a . "&r=1&stay=" . (int)$_POST['stay'] . $add_path;
             } else {
                 $redirectUrl = "index.php?a=3&id=$id&r=1" . $add_path;
             }
diff --git a/manager/processors/save_htmlsnippet.processor.php b/manager/processors/save_htmlsnippet.processor.php
index dafcd8d350..3207d8385c 100755
--- a/manager/processors/save_htmlsnippet.processor.php
+++ b/manager/processors/save_htmlsnippet.processor.php
@@ -97,7 +97,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "78&id=$id" : "77";
-            $header = "Location: index.php?a=" . $a . "&tab=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&tab=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $header = "Location: index.php?a=76&tab=2";
@@ -137,7 +137,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "78&id=$id" : "77";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             evo()->unlockElement(3, $id);
diff --git a/manager/processors/save_module.processor.php b/manager/processors/save_module.processor.php
index 4a4d51ebe6..0ff9fa4e8b 100755
--- a/manager/processors/save_module.processor.php
+++ b/manager/processors/save_module.processor.php
@@ -144,7 +144,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "108&id=$newid" : "107";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $header = "Location: index.php?a=76&tab=5&r=2";
@@ -202,7 +202,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "108&id=$id" : "107";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $modx->unlockElement(6, $id);
diff --git a/manager/processors/save_plugin.processor.php b/manager/processors/save_plugin.processor.php
index b56faa84e7..f4787cb1d6 100755
--- a/manager/processors/save_plugin.processor.php
+++ b/manager/processors/save_plugin.processor.php
@@ -131,7 +131,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "102&id=$newid" : '101';
-            $header = 'Location: index.php?a=' . $a . '&r=2&stay=' . $_POST['stay'];
+            $header = 'Location: index.php?a=' . $a . '&r=2&stay=' . (int)$_POST['stay'];
             header($header);
         } else {
             $header = 'Location: index.php?a=76&tab=4&r=2';
@@ -186,7 +186,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "102&id=$id" : '101';
-            $header = 'Location: index.php?a=' . $a . '&r=2&stay=' . $_POST['stay'];
+            $header = 'Location: index.php?a=' . $a . '&r=2&stay=' . (int)$_POST['stay'];
             header($header);
         } else {
             $modx->unlockElement(5, $id);
diff --git a/manager/processors/save_snippet.processor.php b/manager/processors/save_snippet.processor.php
index a0e9889638..bf33f880f9 100755
--- a/manager/processors/save_snippet.processor.php
+++ b/manager/processors/save_snippet.processor.php
@@ -127,7 +127,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "22&id=$newid" : "23";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $header = "Location: index.php?a=76&tab=3&r=2";
@@ -167,7 +167,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "22&id=$id" : "23";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $modx->unlockElement(4, $id);
diff --git a/manager/processors/save_template.processor.php b/manager/processors/save_template.processor.php
index a241410593..251b977946 100755
--- a/manager/processors/save_template.processor.php
+++ b/manager/processors/save_template.processor.php
@@ -255,7 +255,7 @@ function writeTemplateFile($templatealias, $extension, $content, $mayCreate)
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "16&id=$newid" : "19";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $header = "Location: index.php?a=76&r=2";
@@ -371,7 +371,7 @@ function writeTemplateFile($templatealias, $extension, $content, $mayCreate)
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "16&id=$id" : "19";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             EvolutionCMS()->unlockElement(1, $id);
diff --git a/manager/processors/save_tmplvars.processor.php b/manager/processors/save_tmplvars.processor.php
index f6ddb77e04..49312f1782 100755
--- a/manager/processors/save_tmplvars.processor.php
+++ b/manager/processors/save_tmplvars.processor.php
@@ -99,7 +99,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "301&id=$newid" : "300";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'];
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'];
             header($header);
         } else {
             $header = "Location: index.php?a=76&tab=1&r=2";
@@ -163,7 +163,7 @@
         // finished emptying cache - redirect
         if ($_POST['stay'] != '') {
             $a = ($_POST['stay'] == '2') ? "301&id=$id" : "300";
-            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . $_POST['stay'] . "&or=" . $origin . "&oid=" . $originId;
+            $header = "Location: index.php?a=" . $a . "&r=2&stay=" . (int)$_POST['stay'] . "&or=" . $origin . "&oid=" . $originId;
             header($header);
         } else {
             $modx->unlockElement(2, $id);