Skip to content

Commit

Permalink
[Security] Fix xss in static routes panel (#14947)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@Corepex
Corepex committed Apr 19, 2023
1 parent 1d12840 commit 07a2c95
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 22 deletions.
6 changes: 3 additions & 3 deletions bundles/AdminBundle/Controller/Admin/DataObject/ClassificationstoreController.php
View file
Expand Up @@ -128,7 +128,7 @@ public function deleteGroupAction(Request $request): JsonResponse
*/
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);

Expand All @@ -155,7 +155,7 @@ public function createGroupAction(Request $request): JsonResponse
*/
public function createStoreAction(Request $request): JsonResponse
{
$name = SecurityHelper::getStringWithoutControlChars($request->get('name'));
$name = SecurityHelper::convertHtmlSpecialChars($request->get('name'));

$config = Classificationstore\StoreConfig::getByName($name);

Expand All @@ -181,7 +181,7 @@ public function createStoreAction(Request $request): JsonResponse
*/
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);

Expand Down
11 changes: 11 additions & 0 deletions bundles/AdminBundle/Resources/public/js/pimcore/helpers.js
View file
Expand Up @@ -22,6 +22,17 @@ 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) {
Expand Down
29 changes: 19 additions & 10 deletions bundles/AdminBundle/Resources/public/js/pimcore/settings/staticroutes.js
View file
Expand Up @@ -96,11 +96,11 @@ 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({
Expand All @@ -127,14 +127,17 @@ pimcore.settings.staticroutes = Class.create({
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({
Expand All @@ -143,7 +146,7 @@ pimcore.settings.staticroutes = Class.create({
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,
Expand Down Expand Up @@ -186,9 +189,15 @@ pimcore.settings.staticroutes = Class.create({
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)
}]
}
Expand Down
8 changes: 6 additions & 2 deletions lib/Security/SecurityHelper.php
View file
Expand Up @@ -17,8 +17,12 @@

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;
}
}
17 changes: 10 additions & 7 deletions models/Staticroute.php
View file
Expand Up @@ -17,6 +17,7 @@

use Pimcore\Event\FrontendEvents;
use Pimcore\Model\Exception\NotFoundException;
use Pimcore\Security\SecurityHelper;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
Expand Down Expand Up @@ -292,7 +293,7 @@ public function setId($id)
*/
public function setPattern($pattern)
{
$this->pattern = $pattern;
$this->pattern = SecurityHelper::convertHtmlSpecialChars($pattern);

return $this;
}
Expand All @@ -304,7 +305,7 @@ public function setPattern($pattern)
*/
public function setController($controller)
{
$this->controller = $controller;
$this->controller = SecurityHelper::convertHtmlSpecialChars($controller);

return $this;
}
Expand All @@ -316,7 +317,7 @@ public function setController($controller)
*/
public function setVariables($variables)
{
$this->variables = $variables;
$this->variables = SecurityHelper::convertHtmlSpecialChars($variables);

return $this;
}
Expand All @@ -328,7 +329,7 @@ public function setVariables($variables)
*/
public function setDefaults($defaults)
{
$this->defaults = $defaults;
$this->defaults = SecurityHelper::convertHtmlSpecialChars($defaults);

return $this;
}
Expand Down Expand Up @@ -360,7 +361,7 @@ public function getPriority()
*/
public function setName($name)
{
$this->name = $name;
$this->name = SecurityHelper::convertHtmlSpecialChars($name);

return $this;
}
Expand All @@ -380,7 +381,7 @@ public function getName()
*/
public function setReverse($reverse)
{
$this->reverse = $reverse;
$this->reverse = SecurityHelper::convertHtmlSpecialChars($reverse);

return $this;
}
Expand Down Expand Up @@ -611,7 +612,9 @@ 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;
Expand Down

0 comments on commit 07a2c95

Please sign in to comment.