CVE-2023-38500 (PHP, published 2023-07-25T21:15Z)
Description: 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.
Severity: nan (CVSS v3 base score: 6.1)
Weakness: CWE-79 \u2014 Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')
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.
Repository: https://github.com/TYPO3/html-sanitizer
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 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>

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));
    }