diff --git a/doc/Development_Documentation/03_Documents/01_Editables/18_Link.md b/doc/Development_Documentation/03_Documents/01_Editables/18_Link.md index 7c01e3fa30a..5025dcc0231 100644 --- a/doc/Development_Documentation/03_Documents/01_Editables/18_Link.md +++ b/doc/Development_Documentation/03_Documents/01_Editables/18_Link.md @@ -6,7 +6,7 @@ The link editable is used for dynamic link creation in documents. ## Configuration -You can pass every valid attribute an ``-tag can have ([w3.org - Link](https://www.w3.org/TR/html52/textlevel-semantics.html#the-a-element)), +You can pass nearly every valid attribute an ``-tag can have ([w3.org - Link](https://www.w3.org/TR/html52/textlevel-semantics.html#the-a-element)), such as: `class`, `target`, `id`, `style`, `accesskey`, `name`, `title`, `data-*`, `aria-*` and additionally the following: | Name | Type | Description | @@ -17,6 +17,8 @@ such as: `class`, `target`, `id`, `style`, `accesskey`, `name`, `title`, `data-* | `noText` | boolean | If you need only the `` tag without text (or only with an textSuffix/TextPrefix) | | `required` | boolean/string | (default: false) set to true to make link and text required for publish, set to "linkonly" to make only the link required for publish | +> For security reasons we created an [allow list](https://github.com/pimcore/pimcore/blob/9bf18aca55e5303661c68835c950412a428cf616/models/Document/Editable/Link.php#L115-L141) to filter harmfull html attributes. For example all `on*` attributes will be filtered out! + ## Methods | Name | Return | Description | diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index 3a870e9a793..6dd36a895ca 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -2,6 +2,7 @@ ## 10.5.21 - [Assets] The Asset `Import from Server` feature is now only available for admins. It will be removed in Pimcore 11 +- [Editable] Removed all `on*` attributes from the `$allowedAttributes` list due to security reasons. These attributes are not allowed anymore in the "attributes" field. ## 10.5.13 - [Web2Print] Print document twig expressions are now executed in a sandbox with restrictive security policies (just like Sending mails and Dataobject Text Layouts introduced in 10.5.9). diff --git a/lib/Security/SecurityHelper.php b/lib/Security/SecurityHelper.php index bc1f9f537c4..9deeb9d4dbd 100644 --- a/lib/Security/SecurityHelper.php +++ b/lib/Security/SecurityHelper.php @@ -37,4 +37,13 @@ public static function convertHtmlSpecialCharsArrayKeys(array &$array, array $ke } } } + + public static function sanitizeHtmlAttributes(?string $text): ?string + { + if(is_string($text)) { + return preg_replace('/[\/"\'\\\]/', '', $text); + } + + return null; + } } diff --git a/models/Document/Editable/Link.php b/models/Document/Editable/Link.php index d86fc3202b0..52cf9ee2f3d 100644 --- a/models/Document/Editable/Link.php +++ b/models/Document/Editable/Link.php @@ -19,6 +19,7 @@ use Pimcore\Model; use Pimcore\Model\Asset; use Pimcore\Model\Document; +use Pimcore\Security\SecurityHelper; /** * @method \Pimcore\Model\Document\Editable\Dao getDao() @@ -136,19 +137,7 @@ public function frontend() 'ping', 'type', 'referrerpolicy', - 'xml:lang', - 'onblur', - 'onclick', - 'ondblclick', - 'onfocus', - 'onmousedown', - 'onmousemove', - 'onmouseout', - 'onmouseover', - 'onmouseup', - 'onkeydown', - 'onkeypress', - 'onkeyup', + 'xml:lang' ]; $defaultAttributes = []; @@ -166,9 +155,9 @@ public function frontend() strpos($key, 'aria-') === 0 || in_array($key, $allowedAttributes))) { if (!empty($this->data[$key]) && !empty($this->config[$key])) { - $attribs[] = $key.'="'. $this->data[$key] .' '. $this->config[$key] .'"'; + $attribs[] = $key.'="'. SecurityHelper::sanitizeHtmlAttributes($this->data[$key]) .' '. SecurityHelper::sanitizeHtmlAttributes($this->config[$key]) .'"'; } elseif (!empty($value)) { - $attribs[] = $key.'="'.$value.'"'; + $attribs[] = $key.'="'.SecurityHelper::sanitizeHtmlAttributes($value).'"'; } } } @@ -242,8 +231,7 @@ public function getHref() } if (strlen($this->data['anchor'] ?? '') > 0) { - $anchor = $this->getAnchor(); - $anchor = str_replace('"', urlencode('"'), $anchor); + $anchor = str_replace('"', urlencode('"'), $this->getAnchor()); $url .= '#' . str_replace('#', '', $anchor); } @@ -299,6 +287,10 @@ private function updatePathFromInternal($realPath = false, $editmode = false) } } + if($editmode) { + unset($this->data['attributes']); + } + // sanitize attributes if (isset($this->data['attributes'])) { $this->data['attributes'] = htmlspecialchars($this->data['attributes'], HTML_ENTITIES); @@ -340,7 +332,7 @@ public function getTarget() */ public function getParameters() { - return $this->data['parameters'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['parameters']) ?? ''; } /** @@ -348,7 +340,7 @@ public function getParameters() */ public function getAnchor() { - return $this->data['anchor'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['anchor']) ?? ''; } /** @@ -364,7 +356,7 @@ public function getTitle() */ public function getRel() { - return $this->data['rel'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['rel']) ?? ''; } /** @@ -372,7 +364,7 @@ public function getRel() */ public function getTabindex() { - return $this->data['tabindex'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['tabindex']) ?? ''; } /** @@ -380,7 +372,7 @@ public function getTabindex() */ public function getAccesskey() { - return $this->data['accesskey'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['accesskey']) ?? ''; } /** @@ -388,7 +380,7 @@ public function getAccesskey() */ public function getClass() { - return $this->data['class'] ?? ''; + return SecurityHelper::sanitizeHtmlAttributes($this->data['class']) ?? ''; } /** @@ -409,6 +401,14 @@ public function setDataFromResource($data) $this->data = []; } + //sanitize fields + $fieldsToExclude = ['path']; + foreach($this->data as $key => $value) { + if(!in_array($key, $fieldsToExclude)) { + $this->data[$key] = SecurityHelper::sanitizeHtmlAttributes($value); + } + } + return $this; }