diff --git a/formwork/src/Sanitizer/DomSanitizer.php b/formwork/src/Sanitizer/DomSanitizer.php index 9a3556b35..a950a9317 100644 --- a/formwork/src/Sanitizer/DomSanitizer.php +++ b/formwork/src/Sanitizer/DomSanitizer.php @@ -231,22 +231,30 @@ protected function sanitizeNodeAttributes(DOMElement $domElement): void continue; } - if (!in_array($attribute->nodeName, $this->allowedAttributes, true)) { - $domElement->removeAttribute($attribute->nodeName); - } + $this->sanitizeNodeAttribute($domElement, $attribute); + } + } - if (in_array($attribute->nodeName, $this->uriAttributes, true)) { - $uri = $this->sanitizeUri((string) $attribute->nodeValue); + /** + * Sanitize a single DOM element attribute + */ + protected function sanitizeNodeAttribute(DOMElement $domElement, DOMAttr $domAttr): void + { + if (!in_array($domAttr->nodeName, $this->allowedAttributes, true)) { + $domElement->removeAttribute($domAttr->nodeName); + } - $scheme = Uri::scheme($uri); + if (in_array($domAttr->nodeName, $this->uriAttributes, true)) { + $uri = $this->sanitizeUri((string) $domAttr->nodeValue); - if ($scheme === null && !Str::startsWith($uri, '//')) { - continue; - } + $scheme = Uri::scheme($uri); - if (!$this->allowExternalUris || !in_array($scheme, $this->allowedUriSchemes, true)) { - $domElement->removeAttribute($attribute->nodeName); - } + if ($scheme === null && !Str::startsWith($uri, '//')) { + return; + } + + if (!$this->allowExternalUris || !in_array($scheme, $this->allowedUriSchemes, true)) { + $domElement->removeAttribute($domAttr->nodeName); } } } diff --git a/formwork/src/Sanitizer/Reference/SvgReference.php b/formwork/src/Sanitizer/Reference/SvgReference.php index 239716f63..9666f903e 100644 --- a/formwork/src/Sanitizer/Reference/SvgReference.php +++ b/formwork/src/Sanitizer/Reference/SvgReference.php @@ -318,4 +318,17 @@ class SvgReference 'href', 'xlink:href', ]; + + /** + * SMIL elements that require handling of the `attributeName` attribute to ensure it is safe + * + * @var list + */ + public const array SMIL_ELEMENTS = [ + 'animate', + 'animateMotion', + 'animateTransform', + 'discard', + 'set', + ]; } diff --git a/formwork/src/Sanitizer/SvgSanitizer.php b/formwork/src/Sanitizer/SvgSanitizer.php index 3f0f1645e..66ea8b116 100644 --- a/formwork/src/Sanitizer/SvgSanitizer.php +++ b/formwork/src/Sanitizer/SvgSanitizer.php @@ -19,6 +19,11 @@ class SvgSanitizer extends DomSanitizer protected array $uriAttributes = SvgReference::URI_ATTRIBUTES; + /** + * @var list + */ + protected array $smilElements = SvgReference::SMIL_ELEMENTS; + public function __construct( protected DomParserInterface $domParser = new PhpDomParser(), ) {} @@ -42,6 +47,19 @@ protected function sanitizeDocumentFragment(DOMDocumentFragment $domDocumentFrag $this->addExplicitSvgNamespace($domDocumentFragment); } + protected function sanitizeNodeAttribute(DOMElement $domElement, DOMAttr $domAttr): void + { + parent::sanitizeNodeAttribute($domElement, $domAttr); + + if ( + in_array($domElement->nodeName, $this->smilElements, true) + && $domAttr->name === 'attributeName' + && !$this->isSafeSmilAttributeName($domAttr->value) + ) { + $domElement->removeAttribute($domAttr->name); + } + } + protected function addExplicitSvgNamespace(DOMDocumentFragment $domDocumentFragment): void { $svg = $domDocumentFragment->firstElementChild; @@ -80,4 +98,13 @@ protected function addExplicitSvgNamespace(DOMDocumentFragment $domDocumentFragm $svg->replaceWith($domElement); } + + /** + * Return whether the SMIL `attributeName` is allowed and not a URI attribute (which would be unsafe) + */ + private function isSafeSmilAttributeName(string $attributeName): bool + { + return in_array($attributeName, $this->allowedAttributes, true) + && !in_array($attributeName, $this->uriAttributes, true); + } }