[HtmlSanitizer] Honor universal attribute sanitizers, apply maxInputLength to text contexts, document forceAttribute and allowAttribute caveats

Author: nicolas-grekas

HtmlSanitizer.php

@@ -51,6 +51,11 @@ public function sanitizeFor(string $element, string $input): string
 
     private function sanitizeWithContext(string $context, string $input): string
     {
+        // Prevent DOS attack induced by extremely long HTML strings
+        if (-1 !== $this->config->getMaxInputLength() && \strlen($input) > $this->config->getMaxInputLength()) {
+            $input = substr($input, 0, $this->config->getMaxInputLength());
+        }
+
         // Text context: early return with HTML encoding
         if (W3CReference::CONTEXT_TEXT === $context) {
             return StringSanitizer::encodeHtmlEntities($input);
@@ -59,11 +64,6 @@ private function sanitizeWithContext(string $context, string $input): string
         // Other context: build a DOM visitor
         $this->domVisitors[$context] ??= $this->createDomVisitorForContext($context);
 
-        // Prevent DOS attack induced by extremely long HTML strings
-        if (-1 !== $this->config->getMaxInputLength() && \strlen($input) > $this->config->getMaxInputLength()) {
-            $input = substr($input, 0, $this->config->getMaxInputLength());
-        }
-
         // Only operate on valid UTF-8 strings. This is necessary to prevent cross
         // site scripting issues on Internet Explorer 6. Idea from Drupal (filter_xss).
         if (!$this->isValidUtf8($input)) {

HtmlSanitizerConfig.php

@@ -318,6 +318,11 @@ public function dropElement(string $element): static
      * A list of allowed elements for this attribute can be passed as a second argument.
      * Passing "*" will allow all currently allowed elements to use this attribute.
      *
+     * Note: this method is subtractive within the currently allowed elements.
+     * It restricts the attribute to the listed elements and removes it from any
+     * other allowed element that previously had it. To add an attribute to one
+     * element without affecting others, use allowElement($element, [$attribute]).
+     *
      * @param list<string>|string $allowedElements
      */
     public function allowAttribute(string $attribute, array|string $allowedElements): static
@@ -370,7 +375,10 @@ public function dropAttribute(string $attribute, array|string $droppedElements):
     /**
      * Forcefully set the value of a given attribute on a given element.
      *
-     * The attribute will be created on the nodes if it didn't exist.
+     * The attribute will be created on the nodes if it didn't exist. The
+     * provided value is written verbatim and is NOT passed through any
+     * attribute sanitizer (in particular, URL attribute sanitization is
+     * skipped), so callers are responsible for ensuring the value is safe.
      */
     public function forceAttribute(string $element, string $attribute, string $value): static
     {

Tests/HtmlSanitizerCustomTest.php

@@ -531,6 +531,45 @@ public function sanitizeAttribute(string $element, string $attribute, string $va
         );
     }
 
+    public function testWildcardAttributeSanitizerIsCalled()
+    {
+        $config = (new HtmlSanitizerConfig())
+            ->allowElement('div', ['data-foo', 'data-bar'])
+            ->withAttributeSanitizer(new class implements AttributeSanitizerInterface {
+                public function getSupportedElements(): ?array
+                {
+                    return null;
+                }
+
+                public function getSupportedAttributes(): ?array
+                {
+                    return null;
+                }
+
+                public function sanitizeAttribute(string $element, string $attribute, string $value, HtmlSanitizerConfig $config): ?string
+                {
+                    return strrev($value);
+                }
+            })
+        ;
+
+        $this->assertSame(
+            '<div data-foo="cba" data-bar="zyx">Hello world</div>',
+            $this->sanitize($config, '<div data-foo="abc" data-bar="xyz">Hello world</div>')
+        );
+    }
+
+    public function testMaxInputLengthIsAppliedToTextContext()
+    {
+        $config = (new HtmlSanitizerConfig());
+
+        $input = str_repeat('A', $config->getMaxInputLength() + 100);
+        $expected = str_repeat('A', $config->getMaxInputLength());
+
+        $this->assertSame($expected, (new HtmlSanitizer($config))->sanitizeFor('textarea', $input));
+        $this->assertSame($expected, (new HtmlSanitizer($config))->sanitizeFor('title', $input));
+    }
+
     private function sanitize(HtmlSanitizerConfig $config, string $input): string
     {
         return (new HtmlSanitizer($config))->sanitize($input);

Visitor/DomVisitor.php

@@ -164,6 +164,7 @@ private function setAttributes(string $domNodeName, \DOMNode $domNode, Node $nod
                     $this->attributeSanitizers[$domNodeName][$name] ?? [],
                     $this->attributeSanitizers['*'][$name] ?? [],
                     $this->attributeSanitizers[$domNodeName]['*'] ?? [],
+                    $this->attributeSanitizers['*']['*'] ?? [],
                 );
 
                 foreach ($attributeSanitizers as $sanitizer) {