From 6946f8a5a0a93b516c49f17a5b45044eebd73480 Mon Sep 17 00:00:00 2001 From: robertSt7 <104770750+robertSt7@users.noreply.github.com> Date: Fri, 21 Apr 2023 10:46:57 +0200 Subject: [PATCH] [Security] Stored cross site scripting vulnerability in operator any getter in pimcore grid configuration (#14984) * Fix: xss in anyGetter * Fix: xss for predefined asset metadata --- .../Admin/Asset/AssetHelperController.php | 6 ++++-- .../Admin/DataObject/DataObjectHelperController.php | 8 ++++++++ .../pimcore/object/gridcolumn/operator/AnyGetter.js | 13 ++++++++----- .../js/pimcore/settings/metadata/predefined.js | 2 +- .../GridColumnConfig/Operator/AbstractOperator.php | 5 +++-- .../GridColumnConfig/Operator/AnyGetter.php | 9 +++++---- lib/Security/SecurityHelper.php | 9 +++++++++ 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php b/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php index e7849dc20d0..8701ac111e5 100644 --- a/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php +++ b/bundles/AdminBundle/Controller/Admin/Asset/AssetHelperController.php @@ -33,6 +33,7 @@ use Pimcore\Model\GridConfigShare; use Pimcore\Model\Metadata; use Pimcore\Model\User; +use Pimcore\Security\SecurityHelper; use Pimcore\Tool; use Pimcore\Tool\Storage; use Pimcore\Version; @@ -963,9 +964,10 @@ public function getMetadataForColumnConfigAction(Request $request) if (!in_array($uniqueKey, $tmp) && !in_array($item->getName(), $defaultMetadataNames)) { $tmp[] = $uniqueKey; $item->expand(); + $name = SecurityHelper::convertHtmlSpecialChars($item->getName()); $metadataItems[] = [ - 'title' => $item->getName(), - 'name' => $item->getName(), + 'title' => $name, + 'name' => $name, 'subtype' => $item->getTargetSubtype(), 'datatype' => 'data', 'fieldtype' => $item->getType(), diff --git a/bundles/AdminBundle/Controller/Admin/DataObject/DataObjectHelperController.php b/bundles/AdminBundle/Controller/Admin/DataObject/DataObjectHelperController.php index db28959f4d5..6a3ba048b1f 100644 --- a/bundles/AdminBundle/Controller/Admin/DataObject/DataObjectHelperController.php +++ b/bundles/AdminBundle/Controller/Admin/DataObject/DataObjectHelperController.php @@ -32,6 +32,7 @@ use Pimcore\Model\GridConfigFavourite; use Pimcore\Model\GridConfigShare; use Pimcore\Model\User; +use Pimcore\Security\SecurityHelper; use Pimcore\Tool; use Pimcore\Tool\Storage; use Pimcore\Version; @@ -355,6 +356,13 @@ public function doGetGridColumnConfig(Request $request, Config $config, $isDelet $gridConfigDescription = $savedGridConfig->getDescription(); $sharedGlobally = $savedGridConfig->isShareGlobally(); $setAsFavourite = $savedGridConfig->isSetAsFavourite(); + + foreach($gridConfig['columns'] as &$column) { + if (array_key_exists('isOperator', $column) && $column['isOperator']) { + $colAttributes = &$column['fieldConfig']['attributes']; + SecurityHelper::convertHtmlSpecialCharsArrayKeys($colAttributes, ['label', 'attribute', 'param1']); + } + } } } diff --git a/bundles/AdminBundle/Resources/public/js/pimcore/object/gridcolumn/operator/AnyGetter.js b/bundles/AdminBundle/Resources/public/js/pimcore/object/gridcolumn/operator/AnyGetter.js index 3ee3c4bff73..274279575d9 100644 --- a/bundles/AdminBundle/Resources/public/js/pimcore/object/gridcolumn/operator/AnyGetter.js +++ b/bundles/AdminBundle/Resources/public/js/pimcore/object/gridcolumn/operator/AnyGetter.js @@ -83,22 +83,25 @@ pimcore.object.gridcolumn.operator.anygetter = Class.create(pimcore.object.gridc fieldLabel: t('label'), length: 255, width: 200, - value: this.node.data.configAttributes.label + value: this.node.data.configAttributes.label, + listeners: {'change': pimcore.helpers.htmlEncodeTextField } }); this.attributeField = new Ext.form.TextField({ fieldLabel: t('attribute'), length: 255, width: 200, - value: this.node.data.configAttributes.attribute + value: this.node.data.configAttributes.attribute, + listeners: {'change': pimcore.helpers.htmlEncodeTextField } }); this.param1Field = new Ext.form.TextField({ fieldLabel: t('parameter'), length: 255, width: 200, - value: this.node.data.configAttributes.param1 - }); + value: this.node.data.configAttributes.param1, + listeners: {'change': pimcore.helpers.htmlEncodeTextField } + }); this.returnLastResultField = new Ext.form.Checkbox({ fieldLabel: t('return_last_result'), @@ -183,7 +186,7 @@ pimcore.object.gridcolumn.operator.anygetter = Class.create(pimcore.object.gridc if (configAttributes.param1) { attr += " " + configAttributes.param1; } - nodeLabel += ' (' + attr + ')'; + nodeLabel += ' (' + Ext.util.Format.htmlEncode(attr) + ')'; } return nodeLabel; diff --git a/bundles/AdminBundle/Resources/public/js/pimcore/settings/metadata/predefined.js b/bundles/AdminBundle/Resources/public/js/pimcore/settings/metadata/predefined.js index a7e644ae987..d9f2fa9d8fd 100644 --- a/bundles/AdminBundle/Resources/public/js/pimcore/settings/metadata/predefined.js +++ b/bundles/AdminBundle/Resources/public/js/pimcore/settings/metadata/predefined.js @@ -138,7 +138,7 @@ pimcore.settings.metadata.predefined = Class.create({ sortable: true }, {text: t("name"), width: 200, sortable: true, dataIndex: 'name', - getEditor: function() { return new Ext.form.TextField({}); } + getEditor: function() { return new Ext.form.TextField({ listeners: {'change': pimcore.helpers.htmlEncodeTextField } }); } }, {text: t("group"), width: 200, sortable: true, dataIndex: 'group', getEditor: function() { return new Ext.form.TextField({}); } diff --git a/lib/DataObject/GridColumnConfig/Operator/AbstractOperator.php b/lib/DataObject/GridColumnConfig/Operator/AbstractOperator.php index 47e5545d27a..783170c7c4c 100644 --- a/lib/DataObject/GridColumnConfig/Operator/AbstractOperator.php +++ b/lib/DataObject/GridColumnConfig/Operator/AbstractOperator.php @@ -16,6 +16,7 @@ namespace Pimcore\DataObject\GridColumnConfig\Operator; use Pimcore\DataObject\GridColumnConfig\ConfigElementInterface; +use Pimcore\Security\SecurityHelper; use Pimcore\Tool; abstract class AbstractOperator implements OperatorInterface @@ -41,7 +42,7 @@ abstract class AbstractOperator implements OperatorInterface */ public function __construct(\stdClass $config, array $context = []) { - $this->label = $config->label; + $this->label = SecurityHelper::convertHtmlSpecialChars($config->label); $this->childs = $config->childs; $this->context = $context; } @@ -91,7 +92,7 @@ public function getLabel() */ public function setLabel($label) { - $this->label = $label; + $this->label = SecurityHelper::convertHtmlSpecialChars($label); } /** diff --git a/lib/DataObject/GridColumnConfig/Operator/AnyGetter.php b/lib/DataObject/GridColumnConfig/Operator/AnyGetter.php index ccf970ce46c..6bd0e9ef663 100644 --- a/lib/DataObject/GridColumnConfig/Operator/AnyGetter.php +++ b/lib/DataObject/GridColumnConfig/Operator/AnyGetter.php @@ -16,6 +16,7 @@ namespace Pimcore\DataObject\GridColumnConfig\Operator; use Pimcore\Model\AbstractModel; +use Pimcore\Security\SecurityHelper; use Pimcore\Tool\Admin; /** @@ -64,8 +65,8 @@ public function __construct(\stdClass $config, $context = null) parent::__construct($config, $context); - $this->attribute = $config->attribute ?? ''; - $this->param1 = $config->param1 ?? ''; + $this->attribute = SecurityHelper::convertHtmlSpecialChars($config->attribute ?? ''); + $this->param1 = SecurityHelper::convertHtmlSpecialChars($config->param1 ?? ''); $this->isArrayType = $config->isArrayType ?? false; $this->forwardAttribute = $config->forwardAttribute ?? ''; @@ -182,7 +183,7 @@ public function getAttribute() */ public function setAttribute($attribute) { - $this->attribute = $attribute; + $this->attribute = SecurityHelper::convertHtmlSpecialChars($attribute); } /** @@ -198,7 +199,7 @@ public function getParam1() */ public function setParam1($param1) { - $this->param1 = $param1; + $this->param1 = SecurityHelper::convertHtmlSpecialChars($param1); } /** diff --git a/lib/Security/SecurityHelper.php b/lib/Security/SecurityHelper.php index 541238e2bba..4ba989380af 100644 --- a/lib/Security/SecurityHelper.php +++ b/lib/Security/SecurityHelper.php @@ -25,4 +25,13 @@ public static function convertHtmlSpecialChars(?string $text): ?string return null; } + + public static function convertHtmlSpecialCharsArrayKeys(array &$array, array $keys): void + { + foreach ($keys as $key) { + if (array_key_exists($key, $array)) { + $array[$key] = self::convertHtmlSpecialChars($array[$key]); + } + } + } }