CVE-2023-38500: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: TYPO3 HTML Sanitizer is an HTML sanitizer, written in PHP, aiming to provide cross-site-scripting-safe markup based on explicitly allowed tags, attributes and values. Starting in version 1.0.0 and prior to versions 1.5.1 and 2.1.2, due to an encoding issue in the serialization layer, malicious markup nested in a `noscript` element was not encoded correctly. `noscript` is disabled in the default configuration, but might have been enabled in custom scenarios. This allows bypassing the cross-site scripting mechanism of TYPO3 HTML Sanitizer. Versions 1.5.1 and 2.1.2 fix the problem.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2023-07-25T21:15Z
Repository: https://github.com/TYPO3/html-sanitizer
Language: PHP
Files changed: src/Behavior.php, src/Behavior/CdataSection.php, src/Behavior/Comment.php, src/Builder/CommonBuilder.php, src/Sanitizer.php, src/Serializer/Rules.php, src/Visitor/CommonVisitor.php, tests/BehaviorTest.php, tests/CommonBuilderTest.php, tests/ScenarioTest.php
Commit date: 2023-07-25 09:14:55 +0200
Commit message: Merge pull request from GHSA-59jf-3q9v-rh6g

* [SECURITY] Properly encode noscript child nodes

The `<noscript>` element has a special behavior when
being evaluated in browsers, which depends on whether
script-parsing is enabled or disabled.

As a consequence this change will

* encode comment inner data, e.g. `<!-- <"comment"> -->`
  as `<!-- &lt;&quot;comment&quot;&gt -->`
* always encode tag attributes, e.g. `<p id="<value>">`
  as `<p id="&lt;value&gt;">`
* extend `Comment` and `CdataSection` to have a constructor
  (which triggers encoding per default) and to implement the
  `Behavior\HandlerInterface`
* add a new serializer option `encode_attributes`, which might
  basically be extracted to `Masterminds\HTML5`

* [TASK] Ensure attribute serialization preserves values as is

We don't want so called double-encoding – which is a valid
usecase when HTML describes how HTML is to be written – to
be automagically transformed to single encoded values,
as otherwise a valid input like

<a title="Insert &amp;amp; to write an &amp;"></a>
(Browser would show "Insert &amp; to write an &")

…would be changed to:

<a title="Insert &amp; to write an &amp;"></a>
(Browser would show "Insert & to write an &")

Also add tests for the attribute encoding we want:
 * Encode quotes, tags and stuff that might cause security issues
 * do not encode unnecessarily encode slashes or colons
   (like htmlentitites would do)

---------

Co-authored-by: Benjamin Franzke <ben@bnf.dev>
Diff: diff --git a/src/Behavior.php b/src/Behavior.php
index 1cc88b7..d26c10c 100644
--- a/src/Behavior.php
+++ b/src/Behavior.php
@@ -13,10 +13,12 @@ declare(strict_types=1);
  */
 
 namespace TYPO3\HtmlSanitizer;
 
 use LogicException;
+use TYPO3\HtmlSanitizer\Behavior\CdataSection;
+use TYPO3\HtmlSanitizer\Behavior\Comment;
 use TYPO3\HtmlSanitizer\Behavior\NodeInterface;
 use TYPO3\HtmlSanitizer\Behavior\Tag;
 
 /**
  * Declares behavior used by node visitors
@@ -72,15 +74,20 @@ class Behavior
 
     /**
      * Node names as array index, e.g. `['strong' => new Tag('strong', '#comment' => new Comment()]`
      * @var array<string, ?NodeInterface>
      */
-    protected $nodes = [
+    protected $nodes = [];
+
+    public function __construct()
+    {
         // v2.1.0: adding `#comment` and `#cdata-section` hints for backward compatibility, will be removed with v3.0.0
-        '#comment' => null,
-        '#cdata-section' => null,
-    ];
+        $this->nodes = array_merge($this->nodes, [
+            '#comment' => new Comment(),
+            '#cdata-section' => new CdataSection(),
+        ]);
+    }
 
     public function withFlags(int $flags): self
     {
         if ($flags === $this->flags) {
             return $this;
@@ -123,24 +130,22 @@ class Behavior
         // uses node name as array index, e.g. `['#comment' => new Comment()]`
         $indexedNodes = array_combine($names, $nodes);
         if (!is_array($indexedNodes)) {
             return $this;
         }
-        $this->assertNodeUniqueness($indexedNodes);
         $target = clone $this;
         $target->nodes = array_merge($target->nodes, $indexedNodes);
         return $target;
     }
 
     public function withoutNodes(NodeInterface ...$nodes): self
     {
         $names = array_map([$this, 'getNodeName'], $nodes);
         $filteredNodes = array_filter(
             $this->nodes,
-            static function (?NodeInterface $node, string $name) use ($nodes, $names) {
-                return $node === null && !in_array($name, $names, true)
-                    || $node !== null && !in_array($node, $nodes, true);
+            static function (NodeInterface $node, string $name) use ($nodes, $names) {
+                return !in_array($name, $names, true) && !in_array($node, $nodes, true);
             },
             ARRAY_FILTER_USE_BOTH
         );
         if ($filteredNodes === $this->nodes) {
             return $this;
@@ -245,27 +250,10 @@ class Behavior
                 1625591503
             );
         }
     }
 
-    /**
-     * @param array<string, NodeInterface> $nodes
-     */
-    protected function assertNodeUniqueness(array $nodes): void
-    {
-        $existingNodeNames = array_intersect_key(array_filter($this->nodes), $nodes);
-        if ($existingNodeNames !== []) {
-            throw new LogicException(
-                sprintf(
-                    'Cannot redeclare node names %s. Remove duplicates first',
-                    implode(', ', array_keys($existingNodeNames))
-                ),
-                1625391217
-            );
-        }
-    }
-
     protected function getNodeName(NodeInterface $node): string
     {
         return strtolower($node->getName());
     }
 }
diff --git a/src/Behavior/CdataSection.php b/src/Behavior/CdataSection.php
index df2a432..7640dc1 100644
--- a/src/Behavior/CdataSection.php
+++ b/src/Behavior/CdataSection.php
@@ -12,15 +12,38 @@ declare(strict_types=1);
  * The TYPO3 project - inspiring people to share!
  */
 
 namespace TYPO3\HtmlSanitizer\Behavior;
 
+use DOMNode;
+use DOMText;
+use TYPO3\HtmlSanitizer\Behavior;
+use TYPO3\HtmlSanitizer\Context;
+
 /**
  * Model of CDATA node.
  */
-class CdataSection implements NodeInterface
+class CdataSection implements NodeInterface, HandlerInterface
 {
+    /**
+     * @var bool
+     */
+    protected $secure = true;
+
+    public function __construct(bool $secure = true)
+    {
+        $this->secure = $secure;
+    }
+
     public function getName(): string
     {
         return '#cdata-section';
     }
+
+    public function handle(NodeInterface $node, ?DOMNode $domNode, Context $context, Behavior $behavior = null): ?DOMNode
+    {
+        if (!$this->secure || $domNode === null) {
+            return $domNode;
+        }
+        return new DOMText(trim($domNode->nodeValue));
+    }
 }
diff --git a/src/Behavior/Comment.php b/src/Behavior/Comment.php
index ea798ca..e271943 100644
--- a/src/Behavior/Comment.php
+++ b/src/Behavior/Comment.php
@@ -12,15 +12,38 @@ declare(strict_types=1);
  * The TYPO3 project - inspiring people to share!
  */
 
 namespace TYPO3\HtmlSanitizer\Behavior;
 
+use DOMComment;
+use DOMNode;
+use TYPO3\HtmlSanitizer\Behavior;
+use TYPO3\HtmlSanitizer\Context;
+
 /**
  * Model of comment node.
  */
-class Comment implements NodeInterface
+class Comment implements NodeInterface, HandlerInterface
 {
+    /**
+     * @var bool
+     */
+    protected $secure = true;
+
+    public function __construct(bool $secure = true)
+    {
+        $this->secure = $secure;
+    }
+
     public function getName(): string
     {
         return '#comment';
     }
+
+    public function handle(NodeInterface $node, ?DOMNode $domNode, Context $context, Behavior $behavior = null): ?DOMNode
+    {
+        if (!$this->secure || $domNode === null) {
+            return $domNode;
+        }
+        return new DOMComment(htmlspecialchars($domNode->textContent, ENT_QUOTES, 'UTF-8', false));
+    }
 }
diff --git a/src/Builder/CommonBuilder.php b/src/Builder/CommonBuilder.php
index 16529e2..9e38f08 100644
--- a/src/Builder/CommonBuilder.php
+++ b/src/Builder/CommonBuilder.php
@@ -12,15 +12,12 @@ declare(strict_types=1);
  * The TYPO3 project - inspiring people to share!
  */
 
 namespace TYPO3\HtmlSanitizer\Builder;
 
-use DOMNode;
-use DOMText;
 use TYPO3\HtmlSanitizer\Behavior;
 use TYPO3\HtmlSanitizer\Behavior\Attr\UriAttrValueBuilder;
-use TYPO3\HtmlSanitizer\Behavior\NodeInterface;
 use TYPO3\HtmlSanitizer\Sanitizer;
 use TYPO3\HtmlSanitizer\Visitor\CommonVisitor;
 
 /**
  * Builder, creating a `Sanitizer` instance with "common"
@@ -81,12 +78,11 @@ class CommonBuilder implements BuilderInterface
         return (new Behavior())
             ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
             ->withName('common')
             ->withTags(...array_values($this->createBasicTags()))
             ->withTags(...array_values($this->createMediaTags()))
-            ->withTags(...array_values($this->createTableTags()))
-            ->withNodes(...array_values($this->createSpecialNodes()));
+            ->withTags(...array_values($this->createTableTags()));
     }
 
     protected function createBasicTags(): array
     {
         $names = [
@@ -209,27 +205,10 @@ class CommonBuilder implements BuilderInterface
             ->addAttrs(...$this->globalAttrs, ...$commonTableAttrs)
             ->addAttrs(...$this->createAttrs('span', 'width'));
         return $tags;
     }
 
-    /**
-     * @return array<string, Behavior\NodeInterface>
-     */
-    protected function createSpecialNodes(): array
-    {
-        $nodes = [];
-        $nodes['#cdata-section'] = (new Behavior\NodeHandler(
-            new Behavior\CdataSection(),
-            new Behavior\Handler\ClosureHandler(
-                static function (NodeInterface $node, ?DOMNode $domNode) {
-                    return $domNode !== null ? new DOMText(trim($domNode->nodeValue)) : null;
-                }
-            )
-        ));
-        return $nodes;
-    }
-
     /**
      * @return Behavior\Attr[]
      */
     protected function createGlobalAttrs(): array
     {
diff --git a/src/Sanitizer.php b/src/Sanitizer.php
index d3fb41a..5f299fb 100644
--- a/src/Sanitizer.php
+++ b/src/Sanitizer.php
@@ -39,10 +39,11 @@ use TYPO3\HtmlSanitizer\Visitor\VisitorInterface;
 class Sanitizer
 {
     protected const mastermindsDefaultOptions = [
         // Whether the serializer should aggressively encode all characters as entities.
         'encode_entities' => false,
+        'encode_attributes' => true,
         // Prevents the parser from automatically assigning the HTML5 namespace to the DOM document.
         // (adjusted due to https://github.com/Masterminds/html5-php/issues/181#issuecomment-643767471)
         'disable_html_ns' => true,
     ];
 
diff --git a/src/Serializer/Rules.php b/src/Serializer/Rules.php
index 5888e4a..10ca0b0 100644
--- a/src/Serializer/Rules.php
+++ b/src/Serializer/Rules.php
@@ -43,10 +43,15 @@ class Rules extends OutputRules implements RulesInterface
     /**
      * @var ?InitiatorInterface
      */
     protected $initiator;
 
+    /**
+     * @var bool
+     */
+    protected $encodeAttributes;
+
     /**
      * @param Behavior $behavior
      * @param resource$output
      * @param array $options
      * @return self
@@ -64,10 +69,11 @@ class Rules extends OutputRules implements RulesInterface
      * @param array $options
      */
     public function __construct($output, $options = [])
     {
         $this->options = (array)$options;
+        $this->encodeAttributes = !empty($options['encode_attributes']);
         parent::__construct($output, $this->options);
     }
 
     public function withBehavior(Behavior $behavior): self
     {
@@ -156,10 +162,36 @@ class Rules extends OutputRules implements RulesInterface
         }
         // the potentially insecure case, not encoding node data
         $this->wr($domNode->data);
     }
 
+    protected function enc($text, $attribute = false): string
+    {
+        if ($attribute && $this->encodeAttributes && !$this->encode) {
+            // In contrast to parent::enc() (when $this->encode is true),
+            // we are using htmlspecialchars() instead of htmlentities() as
+            // colons and slashes do not need to be aggressively escaped.
+            $value = htmlspecialchars(
+                $text,
+                ENT_HTML5 | ENT_SUBSTITUTE | ENT_QUOTES,
+                'UTF-8',
+                // $double_encode: true
+                // (name is misleading, it actually means: disable-automagic/always-encode)
+                // Our input is always entity decoded by the parser and we do not
+                // want to consider our input to possibly contain valid entities
+                // we rather want to treat it as pure text, that is *always* to be encoded.
+                true
+            );
+            // htmlspecialchars does escaping, but it doesn't match the requirements of
+            // HTML5 section 8.3 to ecape non breaking spaces
+            // https://www.w3.org/TR/2013/CR-html5-20130806/syntax.html#escapingString
+            $value = implode('&nbsp;', mb_split("\xc2\xa0", $value));
+            return $value;
+        }
+        return parent::enc($text, $attribute);
+    }
+
     /**
      * If the element has a declared namespace in the HTML, MathML or
      * SVG namespaces, we use the localName instead of the tagName.
      */
     protected function resolveNodeName(DOMElement $domNode): string
diff --git a/src/Visitor/CommonVisitor.php b/src/Visitor/CommonVisitor.php
index 7ad0d0d..92c5602 100644
--- a/src/Visitor/CommonVisitor.php
+++ b/src/Visitor/CommonVisitor.php
@@ -84,10 +84,13 @@ class CommonVisitor extends AbstractVisitor implements LoggerAwareInterface
                 $domNode = $this->enterDomElement($domNode, $node->getNode());
             }
             if (!$node->shallHandleFirst()) {
                 $domNode = $node->getHandler()->handle($node->getNode(), $domNode, $this->context, $this->behavior);
             }
+        } elseif ($node instanceof Behavior\HandlerInterface) {
+            $domNode = $node->handle($node, $domNode, $this->context, $this->behavior);
+            $domNode = $domNode instanceof DOMElement ? $this->enterDomElement($domNode, $node) : $domNode;
         } elseif ($domNode instanceof DOMElement) {
             $domNode = $this->enterDomElement($domNode, $node);
         }
         return $domNode;
     }
diff --git a/tests/BehaviorTest.php b/tests/BehaviorTest.php
index 2e33d2e..4575d14 100644
--- a/tests/BehaviorTest.php
+++ b/tests/BehaviorTest.php
@@ -22,11 +22,10 @@ use TYPO3\HtmlSanitizer\Behavior\Tag;
 class BehaviorTest extends TestCase
 {
     public function ambiguityIsDetectedDataProvider(): array
     {
         return [
-            [ ['same'], ['same'], 1625391217 ],
             [ ['same', 'same'], [], 1625591503 ],
             [ ['same', 'same'], ['same'], 1625591503 ],
             [ [], ['same', 'same'], 1625591503 ],
             [ ['same'], ['same', 'same'], 1625591503 ],
         ];
diff --git a/tests/CommonBuilderTest.php b/tests/CommonBuilderTest.php
index 2feb80a..a4ca310 100644
--- a/tests/CommonBuilderTest.php
+++ b/tests/CommonBuilderTest.php
@@ -254,17 +254,25 @@ class CommonBuilderTest extends TestCase
             '#909' => [
                 '<!-- #comment -->',
                 '<!-- #comment -->',
             ],
             '#910' => [
-                '<![CDATA[ #cdata ]]>',
-                '#cdata',
+                '<!-- <"comment"> -->',
+                '<!-- &lt;&quot;comment&quot;&gt; -->',
             ],
             '#911' => [
+                '<!-- &lt;&quot;comment&quot;&gt; -->',
+                '<!-- &lt;&quot;comment&quot;&gt; -->',
+            ],
+            '#915' => [
                 '#text',
                 '#text',
             ],
+            '#920' => [
+                '<![CDATA[ #cdata ]]>',
+                '#cdata',
+            ],
             '#921' => [
                 '<![CDATA[<any><span data-value="value"></any>*/]]>',
                 '&lt;any&gt;&lt;span data-value="value"&gt;&lt;/any&gt;*/',
             ],
             '#930' => [
@@ -285,10 +293,18 @@ class CommonBuilderTest extends TestCase
             ],
             '#934' => [
                 '<img src="/typo3.org/logo.svg"><any>value</any></img>',
                 '<img src="/typo3.org/logo.svg">&lt;any&gt;value&lt;/any&gt;',
             ],
+            '#935' => [
+                '<p class="</p><script>alert(1)">value</p>',
+                '<p class="&lt;/p&gt;&lt;script&gt;alert(1)">value</p>',
+            ],
+            '#936' => [
+                '<p class="{&quot;json&quot;:true}">value</p>',
+                '<p class="{&quot;json&quot;:true}">value</p>',
+            ],
         ];
     }
 
     /**
      * @param string $payload
diff --git a/tests/ScenarioTest.php b/tests/ScenarioTest.php
index 96c94fb..27771be 100644
--- a/tests/ScenarioTest.php
+++ b/tests/ScenarioTest.php
@@ -229,27 +229,45 @@ class ScenarioTest extends TestCase
     public static function commentsAreHandledDataProvider(): array
     {
         return [
             'not allowed' => [
                 false,
+                null,
                 Behavior::BLUNT,
                 '<div><!-- before -->test<!-- after --></div>',
                 '<div>test</div>'
             ],
-            'allowed' => [
+            'allowed, insecure' => [
                 true,
+                false,
                 Behavior::BLUNT,
                 '<div><!-- before -->test<!-- after --></div>',
                 '<div><!-- before -->test<!-- after --></div>'
             ],
-            'not allowed, encoded' => [
+            'allowed, secure' => [
+                true,
+                true,
+                Behavior::BLUNT,
+                '<div><!-- before -->test<!-- after --></div>',
+                '<div><!-- before -->test<!-- after --></div>'
+            ],
+            'not allowed, encode invalid' => [
                 false,
+                null,
                 Behavior::ENCODE_INVALID_COMMENT,
                 '<div><!-- before -->test<!-- after --></div>',
                 '<div>&lt;!-- before --&gt;test&lt;!-- after --&gt;</div>',
             ],
-            'allowed, encoded' => [
+            'allowed, insecure, encode invalid' => [
+                true,
+                false,
+                Behavior::ENCODE_INVALID_COMMENT,
+                '<div><!-- before -->test<!-- after --></div>',
+                '<div><!-- before -->test<!-- after --></div>'
+            ],
+            'allowed, secure, encode invalid' => [
+                true,
                 true,
                 Behavior::ENCODE_INVALID_COMMENT,
                 '<div><!-- before -->test<!-- after --></div>',
                 '<div><!-- before -->test<!-- after --></div>'
             ],
@@ -258,17 +276,17 @@ class ScenarioTest extends TestCase
 
     /**
      * @test
      * @dataProvider commentsAreHandledDataProvider
      */
-    public function commentsAreHandled(bool $allowed, int $flags, string $payload, string $expectation): void
+    public function commentsAreHandled(bool $allowed, ?bool $secure, int $flags, string $payload, string $expectation): void
     {
         $behavior = (new Behavior())
             ->withFlags($flags)
             ->withName('scenario-test')
             ->withTags(new Behavior\Tag('div', Behavior\Tag::ALLOW_CHILDREN));
-        $comment = new Behavior\Comment();
+        $comment = new Behavior\Comment($secure ?? true);
         $behavior = $allowed ? $behavior->withNodes($comment) : $behavior->withoutNodes($comment);
         $sanitizer = new Sanitizer(
             $behavior,
             new CommonVisitor($behavior)
         );
@@ -278,46 +296,64 @@ class ScenarioTest extends TestCase
     public static function cdataSectionsAreHandledDataProvider(): array
     {
         return [
             'not allowed' => [
                 false,
+                null,
                 Behavior::BLUNT,
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
-                '<div>test</div>'
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div>.test.</div>'
             ],
-            'allowed' => [
+            'allowed, insecure' => [
                 true,
+                false,
                 Behavior::BLUNT,
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>'
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>'
             ],
-            'not allowed, encoded' => [
+            'allowed, secure' => [
+                true,
+                true,
+                Behavior::BLUNT,
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div>before.test.after</div>'
+            ],
+            'not allowed, encode invalid' => [
                 false,
+                null,
                 Behavior::ENCODE_INVALID_CDATA_SECTION,
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
-                '<div>&lt;![CDATA[ before ]]&gt;test&lt;![CDATA[ after ]]&gt;</div>',
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div>&lt;![CDATA[ before ]]&gt;.test.&lt;![CDATA[ after ]]&gt;</div>',
             ],
-            'allowed, encoded' => [
+            'allowed, insecure, encode invalid' => [
                 true,
+                false,
                 Behavior::ENCODE_INVALID_CDATA_SECTION,
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
-                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>'
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>'
+            ],
+            'allowed, secure, encode invalid' => [
+                true,
+                true,
+                Behavior::ENCODE_INVALID_CDATA_SECTION,
+                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
+                '<div>before.test.after</div>'
             ],
         ];
     }
 
     /**
      * @test
      * @dataProvider cdataSectionsAreHandledDataProvider
      */
-    public function cdataSectionsAreHandled(bool $allowed, int $flags, string $payload, string $expectation): void
+    public function cdataSectionsAreHandled(bool $allowed, ?bool $secure, int $flags, string $payload, string $expectation): void
     {
         $behavior = (new Behavior())
             ->withFlags($flags)
             ->withName('scenario-test')
             ->withTags(new Behavior\Tag('div', Behavior\Tag::ALLOW_CHILDREN));
-        $cdataSection = new Behavior\CdataSection();
+        $cdataSection = new Behavior\CdataSection($secure ?? true);
         $behavior = $allowed ? $behavior->withNodes($cdataSection) : $behavior->withoutNodes($cdataSection);
         $sanitizer = new Sanitizer(
             $behavior,
             new CommonVisitor($behavior)
         );
@@ -480,6 +516,138 @@ class ScenarioTest extends TestCase
             $behavior,
             new CommonVisitor($behavior)
         );
         self::assertSame($expectation, $sanitizer->sanitize($payload));
     }
+
+    public static function attributesAreEncodedDataProvider(): \Generator
+    {
+        yield 'preserve entities' => [
+	        '<a title="Insert &amp;"></a>',
+	        '<a title="Insert &amp;"></a>',
+        ];
+        yield 'encode single quotes' => [
+	        '<a title="\'"></a>',
+	        '<a title="&apos;"></a>',
+        ];
+        yield 'encode single quotes from entity' => [
+	        '<a title="&#039;"></a>',
+	        '<a title="&apos;"></a>',
+        ];
+        yield 'encode double quotes' => [
+	        "<a title='" . '"' . "'></a>",
+	        '<a title="&quot;"></a>',
+        ];
+        yield 'preserve double quote encoding' => [
+	        '<a title="&quot;"></a>',
+	        '<a title="&quot;"></a>',
+        ];
+        yield 'preserve double encoded entities' => [
+	        '<a title="Insert &amp;amp; to write an &amp;"></a>',
+	        '<a title="Insert &amp;amp; to write an &amp;"></a>',
+        ];
+        yield 'preserve URLs without "agressive" entity encoding' => [
+	        '<a title="https://example.com/"></a>',
+	        '<a title="https://example.com/"></a>',
+        ];
+        yield 'encode tag specifiers' => [
+            '<a id="</noscript><script>alert(1)</script>"></a>',
+            '<a id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></a>',
+        ];
+        // Invalid input seems to be removed during parsing step (where?)
+        // therefore ENT_SUBSTITUTE can not operate during serialization
+        // @todo: check masterminds/html5-php whether that behavior is
+        // intended
+        //yield 'substitute invalid unicode in attributes' => [
+        //    "<a title='Hello \x80, Good morning'></a>",
+        //    "<a title='Hello \xEF\xBF\xBD, Good morning'></a>",
+        //];
+        yield 'escape non breaking space' => [
+            "<a title='Hello\xc2\xa0World'></a>",
+            '<a title="Hello&nbsp;World"></a>',
+        ];
+        yield 'encodes json values' => [
+            "<div data-value='{\"Hello\":[{\"w\":\"o\",\"r\":\"ld\"}]}'></a>",
+            '<div data-value="{&quot;Hello&quot;:[{&quot;w&quot;:&quot;o&quot;,&quot;r&quot;:&quot;ld&quot;}]}"></div>'
+        ];
+        yield 'encodes json values containing html' => [
+            "<div data-value='{\"Hello\":\"&lt;span&gt;World&lt;\/span&gt;\"}'></div>",
+            '<div data-value="{&quot;Hello&quot;:&quot;&lt;span&gt;World&lt;\/span&gt;&quot;}"></div>'
+        ];
+
+    }
+
+    /**
+     * @test
+     * @dataProvider attributesAreEncodedDataProvider
+     */
+    public function attributesAreEncoded(string $payload, string $expectation): void
+    {
+        $behavior = (new Behavior())
+            ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
+            ->withName('scenario-test')
+            ->withTags(
+                (new Behavior\Tag('a', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
+                    new Behavior\Attr('id'),
+                    new Behavior\Attr('title')
+                ),
+                (new Behavior\Tag('div', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
+                    new Behavior\Attr('data-value')
+                )
+            );
+
+        $sanitizer = new Sanitizer(
+            $behavior,
+            new CommonVisitor($behavior)
+        );
+        self::assertSame($expectation, $sanitizer->sanitize($payload));
+    }
+
+    public static function specialTagsAreHandledDataProvider(): \Generator
+    {
+        yield 'noscript attribute' => [
+            '<noscript><p id="</noscript><script>alert(1)</script>"></p>',
+            '<noscript><p id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></p></noscript>',
+        ];
+        yield 'noscript namespaced attribute' => [
+            '<noscript><p test:id="</noscript><script>alert(1)</script>"></p>',
+            '<noscript><p test:id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></p></noscript>',
+        ];
+        yield 'noscript comment' => [
+            '<noscript><!--</noscript><script>alert(2)</script>--></noscript>',
+            '<noscript><!--&lt;/noscript&gt;&lt;script&gt;alert(2)&lt;/script&gt;--></noscript>',
+        ];
+        yield 'noscript raw text' => [
+            '<noscript><style></noscript><script>alert(3)</script>',
+            '<noscript><style>&lt;/noscript&gt;&lt;script&gt;alert(3)&lt;/script&gt;</style></noscript>',
+        ];
+        yield 'noscript event attribute' => [
+            '<noscript><p onmouseover="alert(4)">value</p></noscript>',
+            '<noscript><p>value</p></noscript>',
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider specialTagsAreHandledDataProvider
+     */
+    public function specialTagsAreHandled(string $payload, string $expectation): void
+    {
+        $behavior = (new Behavior())
+            ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
+            ->withName('scenario-test')
+            ->withTags(
+                (new Behavior\Tag('style', Behavior\Tag::ALLOW_CHILDREN)),
+                (new Behavior\Tag('noscript', Behavior\Tag::ALLOW_CHILDREN)),
+                (new Behavior\Tag('p', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
+                    new Behavior\Attr('id'),
+                    new Behavior\Attr('test:id')
+                )
+            );
+
+        $sanitizer = new Sanitizer(
+            $behavior,
+            new CommonVisitor($behavior)
+        );
+        self::assertSame($expectation, $sanitizer->sanitize($payload));
+    }
 }

Vulnerable code:     protected $nodes = [
        '#comment' => null,
        '#cdata-section' => null,
    ];
        $this->assertNodeUniqueness($indexedNodes);
            static function (?NodeInterface $node, string $name) use ($nodes, $names) {
                return $node === null && !in_array($name, $names, true)
                    || $node !== null && !in_array($node, $nodes, true);
    /**
     * @param array<string, NodeInterface> $nodes
     */
    protected function assertNodeUniqueness(array $nodes): void
    {
        $existingNodeNames = array_intersect_key(array_filter($this->nodes), $nodes);
        if ($existingNodeNames !== []) {
            throw new LogicException(
                sprintf(
                    'Cannot redeclare node names %s. Remove duplicates first',
                    implode(', ', array_keys($existingNodeNames))
                ),
                1625391217
            );
        }
    }

class CdataSection implements NodeInterface
class Comment implements NodeInterface
use DOMNode;
use DOMText;
use TYPO3\HtmlSanitizer\Behavior\NodeInterface;
            ->withTags(...array_values($this->createTableTags()))
            ->withNodes(...array_values($this->createSpecialNodes()));
    /**
     * @return array<string, Behavior\NodeInterface>
     */
    protected function createSpecialNodes(): array
    {
        $nodes = [];
        $nodes['#cdata-section'] = (new Behavior\NodeHandler(
            new Behavior\CdataSection(),
            new Behavior\Handler\ClosureHandler(
                static function (NodeInterface $node, ?DOMNode $domNode) {
                    return $domNode !== null ? new DOMText(trim($domNode->nodeValue)) : null;
                }
            )
        ));
        return $nodes;
    }

            [ ['same'], ['same'], 1625391217 ],
                '<![CDATA[ #cdata ]]>',
                '#cdata',
            'allowed' => [
            'not allowed, encoded' => [
            'allowed, encoded' => [
    public function commentsAreHandled(bool $allowed, int $flags, string $payload, string $expectation): void
        $comment = new Behavior\Comment();
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
                '<div>test</div>'
            'allowed' => [
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>'
            'not allowed, encoded' => [
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
                '<div>&lt;![CDATA[ before ]]&gt;test&lt;![CDATA[ after ]]&gt;</div>',
            'allowed, encoded' => [
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>',
                '<div><![CDATA[ before ]]>test<![CDATA[ after ]]></div>'
    public function cdataSectionsAreHandled(bool $allowed, int $flags, string $payload, string $expectation): void
        $cdataSection = new Behavior\CdataSection();
Fixed code: use TYPO3\HtmlSanitizer\Behavior\CdataSection;
use TYPO3\HtmlSanitizer\Behavior\Comment;
    protected $nodes = [];

    public function __construct()
    {
        $this->nodes = array_merge($this->nodes, [
            '#comment' => new Comment(),
            '#cdata-section' => new CdataSection(),
        ]);
    }
            static function (NodeInterface $node, string $name) use ($nodes, $names) {
                return !in_array($name, $names, true) && !in_array($node, $nodes, true);
use DOMNode;
use DOMText;
use TYPO3\HtmlSanitizer\Behavior;
use TYPO3\HtmlSanitizer\Context;

class CdataSection implements NodeInterface, HandlerInterface
    /**
     * @var bool
     */
    protected $secure = true;

    public function __construct(bool $secure = true)
    {
        $this->secure = $secure;
    }


    public function handle(NodeInterface $node, ?DOMNode $domNode, Context $context, Behavior $behavior = null): ?DOMNode
    {
        if (!$this->secure || $domNode === null) {
            return $domNode;
        }
        return new DOMText(trim($domNode->nodeValue));
    }
use DOMComment;
use DOMNode;
use TYPO3\HtmlSanitizer\Behavior;
use TYPO3\HtmlSanitizer\Context;

class Comment implements NodeInterface, HandlerInterface
    /**
     * @var bool
     */
    protected $secure = true;

    public function __construct(bool $secure = true)
    {
        $this->secure = $secure;
    }


    public function handle(NodeInterface $node, ?DOMNode $domNode, Context $context, Behavior $behavior = null): ?DOMNode
    {
        if (!$this->secure || $domNode === null) {
            return $domNode;
        }
        return new DOMComment(htmlspecialchars($domNode->textContent, ENT_QUOTES, 'UTF-8', false));
    }
            ->withTags(...array_values($this->createTableTags()));
        'encode_attributes' => true,
    /**
     * @var bool
     */
    protected $encodeAttributes;

        $this->encodeAttributes = !empty($options['encode_attributes']);
    protected function enc($text, $attribute = false): string
    {
        if ($attribute && $this->encodeAttributes && !$this->encode) {
            // In contrast to parent::enc() (when $this->encode is true),
            // we are using htmlspecialchars() instead of htmlentities() as
            // colons and slashes do not need to be aggressively escaped.
            $value = htmlspecialchars(
                $text,
                ENT_HTML5 | ENT_SUBSTITUTE | ENT_QUOTES,
                'UTF-8',
                // $double_encode: true
                // (name is misleading, it actually means: disable-automagic/always-encode)
                // Our input is always entity decoded by the parser and we do not
                // want to consider our input to possibly contain valid entities
                // we rather want to treat it as pure text, that is *always* to be encoded.
                true
            );
            // htmlspecialchars does escaping, but it doesn't match the requirements of
            // HTML5 section 8.3 to ecape non breaking spaces
            // https://www.w3.org/TR/2013/CR-html5-20130806/syntax.html#escapingString
            $value = implode('&nbsp;', mb_split("\xc2\xa0", $value));
            return $value;
        }
        return parent::enc($text, $attribute);
    }

        } elseif ($node instanceof Behavior\HandlerInterface) {
            $domNode = $node->handle($node, $domNode, $this->context, $this->behavior);
            $domNode = $domNode instanceof DOMElement ? $this->enterDomElement($domNode, $node) : $domNode;
                '<!-- <"comment"> -->',
                '<!-- &lt;&quot;comment&quot;&gt; -->',
                '<!-- &lt;&quot;comment&quot;&gt; -->',
                '<!-- &lt;&quot;comment&quot;&gt; -->',
            ],
            '#915' => [
            '#920' => [
                '<![CDATA[ #cdata ]]>',
                '#cdata',
            ],
            '#935' => [
                '<p class="</p><script>alert(1)">value</p>',
                '<p class="&lt;/p&gt;&lt;script&gt;alert(1)">value</p>',
            ],
            '#936' => [
                '<p class="{&quot;json&quot;:true}">value</p>',
                '<p class="{&quot;json&quot;:true}">value</p>',
            ],
                null,
            'allowed, insecure' => [
                false,
            'allowed, secure' => [
                true,
                true,
                Behavior::BLUNT,
                '<div><!-- before -->test<!-- after --></div>',
                '<div><!-- before -->test<!-- after --></div>'
            ],
            'not allowed, encode invalid' => [
                null,
            'allowed, insecure, encode invalid' => [
                true,
                false,
                Behavior::ENCODE_INVALID_COMMENT,
                '<div><!-- before -->test<!-- after --></div>',
                '<div><!-- before -->test<!-- after --></div>'
            ],
            'allowed, secure, encode invalid' => [
                true,
    public function commentsAreHandled(bool $allowed, ?bool $secure, int $flags, string $payload, string $expectation): void
        $comment = new Behavior\Comment($secure ?? true);
                null,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div>.test.</div>'
            'allowed, insecure' => [
                false,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>'
            'allowed, secure' => [
                true,
                true,
                Behavior::BLUNT,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div>before.test.after</div>'
            ],
            'not allowed, encode invalid' => [
                null,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div>&lt;![CDATA[ before ]]&gt;.test.&lt;![CDATA[ after ]]&gt;</div>',
            'allowed, insecure, encode invalid' => [
                false,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>'
            ],
            'allowed, secure, encode invalid' => [
                true,
                true,
                Behavior::ENCODE_INVALID_CDATA_SECTION,
                '<div><![CDATA[ before ]]>.test.<![CDATA[ after ]]></div>',
                '<div>before.test.after</div>'
    public function cdataSectionsAreHandled(bool $allowed, ?bool $secure, int $flags, string $payload, string $expectation): void
        $cdataSection = new Behavior\CdataSection($secure ?? true);

    public static function attributesAreEncodedDataProvider(): \Generator
    {
        yield 'preserve entities' => [
	        '<a title="Insert &amp;"></a>',
	        '<a title="Insert &amp;"></a>',
        ];
        yield 'encode single quotes' => [
	        '<a title="\'"></a>',
	        '<a title="&apos;"></a>',
        ];
        yield 'encode single quotes from entity' => [
	        '<a title="&#039;"></a>',
	        '<a title="&apos;"></a>',
        ];
        yield 'encode double quotes' => [
	        "<a title='" . '"' . "'></a>",
	        '<a title="&quot;"></a>',
        ];
        yield 'preserve double quote encoding' => [
	        '<a title="&quot;"></a>',
	        '<a title="&quot;"></a>',
        ];
        yield 'preserve double encoded entities' => [
	        '<a title="Insert &amp;amp; to write an &amp;"></a>',
	        '<a title="Insert &amp;amp; to write an &amp;"></a>',
        ];
        yield 'preserve URLs without "agressive" entity encoding' => [
	        '<a title="https://example.com/"></a>',
	        '<a title="https://example.com/"></a>',
        ];
        yield 'encode tag specifiers' => [
            '<a id="</noscript><script>alert(1)</script>"></a>',
            '<a id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></a>',
        ];
        // Invalid input seems to be removed during parsing step (where?)
        // therefore ENT_SUBSTITUTE can not operate during serialization
        // @todo: check masterminds/html5-php whether that behavior is
        // intended
        //yield 'substitute invalid unicode in attributes' => [
        //    "<a title='Hello \x80, Good morning'></a>",
        //    "<a title='Hello \xEF\xBF\xBD, Good morning'></a>",
        //];
        yield 'escape non breaking space' => [
            "<a title='Hello\xc2\xa0World'></a>",
            '<a title="Hello&nbsp;World"></a>',
        ];
        yield 'encodes json values' => [
            "<div data-value='{\"Hello\":[{\"w\":\"o\",\"r\":\"ld\"}]}'></a>",
            '<div data-value="{&quot;Hello&quot;:[{&quot;w&quot;:&quot;o&quot;,&quot;r&quot;:&quot;ld&quot;}]}"></div>'
        ];
        yield 'encodes json values containing html' => [
            "<div data-value='{\"Hello\":\"&lt;span&gt;World&lt;\/span&gt;\"}'></div>",
            '<div data-value="{&quot;Hello&quot;:&quot;&lt;span&gt;World&lt;\/span&gt;&quot;}"></div>'
        ];

    }

    /**
     * @test
     * @dataProvider attributesAreEncodedDataProvider
     */
    public function attributesAreEncoded(string $payload, string $expectation): void
    {
        $behavior = (new Behavior())
            ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
            ->withName('scenario-test')
            ->withTags(
                (new Behavior\Tag('a', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
                    new Behavior\Attr('id'),
                    new Behavior\Attr('title')
                ),
                (new Behavior\Tag('div', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
                    new Behavior\Attr('data-value')
                )
            );

        $sanitizer = new Sanitizer(
            $behavior,
            new CommonVisitor($behavior)
        );
        self::assertSame($expectation, $sanitizer->sanitize($payload));
    }

    public static function specialTagsAreHandledDataProvider(): \Generator
    {
        yield 'noscript attribute' => [
            '<noscript><p id="</noscript><script>alert(1)</script>"></p>',
            '<noscript><p id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></p></noscript>',
        ];
        yield 'noscript namespaced attribute' => [
            '<noscript><p test:id="</noscript><script>alert(1)</script>"></p>',
            '<noscript><p test:id="&lt;/noscript&gt;&lt;script&gt;alert(1)&lt;/script&gt;"></p></noscript>',
        ];
        yield 'noscript comment' => [
            '<noscript><!--</noscript><script>alert(2)</script>--></noscript>',
            '<noscript><!--&lt;/noscript&gt;&lt;script&gt;alert(2)&lt;/script&gt;--></noscript>',
        ];
        yield 'noscript raw text' => [
            '<noscript><style></noscript><script>alert(3)</script>',
            '<noscript><style>&lt;/noscript&gt;&lt;script&gt;alert(3)&lt;/script&gt;</style></noscript>',
        ];
        yield 'noscript event attribute' => [
            '<noscript><p onmouseover="alert(4)">value</p></noscript>',
            '<noscript><p>value</p></noscript>',
        ];
    }

    /**
     * @test
     * @dataProvider specialTagsAreHandledDataProvider
     */
    public function specialTagsAreHandled(string $payload, string $expectation): void
    {
        $behavior = (new Behavior())
            ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
            ->withName('scenario-test')
            ->withTags(
                (new Behavior\Tag('style', Behavior\Tag::ALLOW_CHILDREN)),
                (new Behavior\Tag('noscript', Behavior\Tag::ALLOW_CHILDREN)),
                (new Behavior\Tag('p', Behavior\Tag::ALLOW_CHILDREN))->addAttrs(
                    new Behavior\Attr('id'),
                    new Behavior\Attr('test:id')
                )
            );

        $sanitizer = new Sanitizer(
            $behavior,
            new CommonVisitor($behavior)
        );
        self::assertSame($expectation, $sanitizer->sanitize($payload));
    }