CVE-2023-2616: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site Scripting (XSS) - Generic in GitHub repository pimcore/pimcore prior to 10.5.21.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2023-05-10T05:15Z
Repository: https://github.com/pimcore/pimcore
Language: PHP
Files changed: bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php, bundles/AdminBundle/Resources/public/js/pimcore/helpers.js, bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js, lib/Security/SecurityHelper.php, models/Staticroute.php
Commit date: 2023-04-19 11:35:21 +0200
Commit message: [Security] Fix xss in static routes panel (#14947)

* added new helper function to htmlEncode textFields, fixed xss issue in static routes panel

* added `htmlspecialchars` to model

* fixed double encode issue

* fixed double encode problem in BE

* changed type of `htmlspecialchars`, added `htmlspecialchars` to `setMethods`

* improved `htmlSpecialChars`

* changed SecurityHelper method name to `convertHtmlSpecialChars`

* added htmlEncode to `deleteConfirm` msgbox
Diff: diff --git a/bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php b/bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php
index 6d29374d8a8f..597650e003c1 100644
--- a/bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php
+++ b/bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php
@@ -126,11 +126,11 @@ class ClassificationstoreController extends AdminController implements KernelCon
      *
      * @throws \Exception
      */
     public function createGroupAction(Request $request): JsonResponse
     {
-        $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
+        $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
         $storeId = $request->get('storeId');
         $config = Classificationstore\GroupConfig::getByName($name, $storeId);
 
         if (!$config) {
             $config = new Classificationstore\GroupConfig();
@@ -153,11 +153,11 @@ class ClassificationstoreController extends AdminController implements KernelCon
      *
      * @throws \Exception
      */
     public function createStoreAction(Request $request): JsonResponse
     {
-        $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
+        $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
 
         $config = Classificationstore\StoreConfig::getByName($name);
 
         if (!$config) {
             $config = new Classificationstore\StoreConfig();
@@ -179,11 +179,11 @@ class ClassificationstoreController extends AdminController implements KernelCon
      *
      * @throws \Exception
      */
     public function createCollectionAction(Request $request): JsonResponse
     {
-        $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
+        $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
         $storeId = $request->get('storeId');
         $config = Classificationstore\CollectionConfig::getByName($name, $storeId);
 
         if (!$config) {
             $config = new Classificationstore\CollectionConfig();
diff --git a/bundles/AdminBundle/Resources/public/js/pimcore/helpers.js b/bundles/AdminBundle/Resources/public/js/pimcore/helpers.js
index 63778fd39d91..c77adc323001 100644
--- a/bundles/AdminBundle/Resources/public/js/pimcore/helpers.js
+++ b/bundles/AdminBundle/Resources/public/js/pimcore/helpers.js
@@ -20,10 +20,21 @@ pimcore.helpers.sanitizeEmail = function (email) {
 
 pimcore.helpers.sanitizeUrlSlug = function (slug) {
     return slug.replace(/[^a-z0-9-_+/]/gi, '');
 };
 
+pimcore.helpers.htmlEncodeTextField = function (textField) {
+    if(textField.getValue()) {
+        textField.suspendEvent('change');
+        const decodedValue = Ext.util.Format.htmlDecode(textField.getValue());
+        textField.setValue(
+            Ext.util.Format.htmlEncode(decodedValue)
+        );
+        textField.resumeEvent('change');
+    }
+};
+
 pimcore.helpers.registerKeyBindings = function (bindEl, ExtJS) {
 
     if (!ExtJS) {
         ExtJS = Ext;
     }
diff --git a/bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js b/bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js
index 3dcbbbb99cd7..8feda63cc644 100644
--- a/bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js
+++ b/bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js
@@ -94,15 +94,15 @@ pimcore.settings.staticroutes = Class.create({
             }
         });
 
         var typesColumns = [
             {text:t("name"), flex:50, sortable:true, dataIndex:'name',
-                editor:new Ext.form.TextField({})},
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
             {text:t("pattern"), flex:100, sortable:true, dataIndex:'pattern',
-                editor:new Ext.form.TextField({})},
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
             {text:t("reverse"), flex:100, sortable:true, dataIndex:'reverse',
-                editor:new Ext.form.TextField({})},
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
             {text:t("controller"), flex:200, sortable:false, dataIndex:'controller',
                 editor:new Ext.form.ComboBox({
                     store:new Ext.data.JsonStore({
                         autoDestroy:true,
                         autoLoad: true,
@@ -125,27 +125,30 @@ pimcore.settings.staticroutes = Class.create({
                     triggerAction:"all",
                     displayField:'name',
                     valueField:'name',
                     listConfig: {
                         maxWidth: 400
+                    },
+                    listeners: {
+                        'change': pimcore.helpers.htmlEncodeTextField
                     }
                 })},
             {text:t("variables"), flex:50, sortable:false, dataIndex:'variables',
-                editor:new Ext.form.TextField({})},
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
             {text:t("defaults"), flex:50, sortable:false, dataIndex:'defaults',
-                editor:new Ext.form.TextField({})},
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
             {text:t("site_ids"), flex:100, sortable:true, dataIndex:"siteId",
-                editor:new Ext.form.TextField({}),
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}}),
                 tooltip: t("site_ids_tooltip")
             },
             {text:t("priority"), flex:50, sortable:true, dataIndex:'priority', editor:new Ext.form.ComboBox({
                 store:[1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
                 mode:"local",
                 triggerAction:"all"
             })},
             {text:t("methods"), flex:50, sortable:false, dataIndex:'methods',
-                editor:new Ext.form.TextField({}),
+                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}}),
             },
             {text: t("creationDate"), sortable: true, dataIndex: 'creationDate', editable: false,
                 hidden: true,
                 renderer: function(d) {
                     if (d !== undefined) {
@@ -184,13 +187,19 @@ pimcore.settings.staticroutes = Class.create({
                         var data = grid.getStore().getAt(rowIndex);
                         if (!data.data.writeable) {
                             return;
                         }
 
-                        pimcore.helpers.deleteConfirm(t('staticroute'), data.data.name, function () {
-                            grid.getStore().removeAt(rowIndex);
-                        }.bind(this));
+                        const decodedName = Ext.util.Format.htmlDecode(data.data.name);
+
+                        pimcore.helpers.deleteConfirm(
+                            t('staticroute'),
+                            Ext.util.Format.htmlEncode(decodedName),
+                            function () {
+                                grid.getStore().removeAt(rowIndex);
+                            }.bind(this)
+                        );
                     }.bind(this)
                 }]
             }
         ];
 
diff --git a/lib/Security/SecurityHelper.php b/lib/Security/SecurityHelper.php
index 0bfd3a42b70d..e81d04d7e054 100644
--- a/lib/Security/SecurityHelper.php
+++ b/lib/Security/SecurityHelper.php
@@ -15,10 +15,14 @@
 
 namespace Pimcore\Security;
 
 class SecurityHelper
 {
-    public static function getStringWithoutControlChars(string $text): string
+    public static function convertHtmlSpecialChars(?string $text): ?string
     {
-        return preg_replace('[\\\\<>"\'`!?/%$(){};,:|=]', '', $text);
+        if(is_string($text)){
+            return htmlspecialchars($text, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8', false);
+        }
+
+        return null;
     }
 }
diff --git a/models/Staticroute.php b/models/Staticroute.php
index 983379b303af..2e4dcc655265 100644
--- a/models/Staticroute.php
+++ b/models/Staticroute.php
@@ -15,10 +15,11 @@
 
 namespace Pimcore\Model;
 
 use Pimcore\Event\FrontendEvents;
 use Pimcore\Model\Exception\NotFoundException;
+use Pimcore\Security\SecurityHelper;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
  * @method bool isWriteable()
  * @method string getWriteTarget()
@@ -290,11 +291,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setPattern($pattern)
     {
-        $this->pattern = $pattern;
+        $this->pattern = SecurityHelper::convertHtmlSpecialChars($pattern);
 
         return $this;
     }
 
     /**
@@ -302,11 +303,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setController($controller)
     {
-        $this->controller = $controller;
+        $this->controller = SecurityHelper::convertHtmlSpecialChars($controller);
 
         return $this;
     }
 
     /**
@@ -314,11 +315,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setVariables($variables)
     {
-        $this->variables = $variables;
+        $this->variables = SecurityHelper::convertHtmlSpecialChars($variables);
 
         return $this;
     }
 
     /**
@@ -326,11 +327,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setDefaults($defaults)
     {
-        $this->defaults = $defaults;
+        $this->defaults = SecurityHelper::convertHtmlSpecialChars($defaults);
 
         return $this;
     }
 
     /**
@@ -358,11 +359,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setName($name)
     {
-        $this->name = $name;
+        $this->name = SecurityHelper::convertHtmlSpecialChars($name);
 
         return $this;
     }
 
     /**
@@ -378,11 +379,11 @@ final class Staticroute extends AbstractModel
      *
      * @return $this
      */
     public function setReverse($reverse)
     {
-        $this->reverse = $reverse;
+        $this->reverse = SecurityHelper::convertHtmlSpecialChars($reverse);
 
         return $this;
     }
 
     /**
@@ -609,11 +610,13 @@ final class Staticroute extends AbstractModel
      */
     public function setMethods($methods)
     {
         if (is_string($methods)) {
             $methods = strlen($methods) ? explode(',', $methods) : [];
-            $methods = array_map('trim', $methods);
+            foreach($methods as $key => $method) {
+                $methods[$key] = SecurityHelper::convertHtmlSpecialChars(trim($method));
+            }
         }
 
         $this->methods = $methods;
 
         return $this;

Vulnerable code:         $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
        $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
        $name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
                editor:new Ext.form.TextField({})},
                editor:new Ext.form.TextField({})},
                editor:new Ext.form.TextField({})},
                editor:new Ext.form.TextField({})},
                editor:new Ext.form.TextField({})},
                editor:new Ext.form.TextField({}),
                editor:new Ext.form.TextField({}),
                        pimcore.helpers.deleteConfirm(t('staticroute'), data.data.name, function () {
                            grid.getStore().removeAt(rowIndex);
                        }.bind(this));
    public static function getStringWithoutControlChars(string $text): string
        return preg_replace('[\\\\<>"\'`!?/%$(){};,:|=]', '', $text);
        $this->pattern = $pattern;
        $this->controller = $controller;
        $this->variables = $variables;
        $this->defaults = $defaults;
        $this->name = $name;
        $this->reverse = $reverse;
            $methods = array_map('trim', $methods);
Fixed code:         $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
        $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
        $name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));
pimcore.helpers.htmlEncodeTextField = function (textField) {
    if(textField.getValue()) {
        textField.suspendEvent('change');
        const decodedValue = Ext.util.Format.htmlDecode(textField.getValue());
        textField.setValue(
            Ext.util.Format.htmlEncode(decodedValue)
        );
        textField.resumeEvent('change');
    }
};

                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
                    },
                    listeners: {
                        'change': pimcore.helpers.htmlEncodeTextField
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}})},
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}}),
                editor:new Ext.form.TextField({listeners: {'change': pimcore.helpers.htmlEncodeTextField}}),
                        const decodedName = Ext.util.Format.htmlDecode(data.data.name);

                        pimcore.helpers.deleteConfirm(
                            t('staticroute'),
                            Ext.util.Format.htmlEncode(decodedName),
                            function () {
                                grid.getStore().removeAt(rowIndex);
                            }.bind(this)
                        );
    public static function convertHtmlSpecialChars(?string $text): ?string
        if(is_string($text)){
            return htmlspecialchars($text, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8', false);
        }

        return null;
use Pimcore\Security\SecurityHelper;
        $this->pattern = SecurityHelper::convertHtmlSpecialChars($pattern);
        $this->controller = SecurityHelper::convertHtmlSpecialChars($controller);
        $this->variables = SecurityHelper::convertHtmlSpecialChars($variables);
        $this->defaults = SecurityHelper::convertHtmlSpecialChars($defaults);
        $this->name = SecurityHelper::convertHtmlSpecialChars($name);
        $this->reverse = SecurityHelper::convertHtmlSpecialChars($reverse);
            foreach($methods as $key => $method) {
                $methods[$key] = SecurityHelper::convertHtmlSpecialChars(trim($method));
            }