CVE-2023-47125: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: TYPO3 is an open source PHP based web content management system released under the GNU GPL. In affected versions DOM processing instructions are not handled correctly. This allows bypassing the cross-site scripting mechanism of typo3/html-sanitizer. This vulnerability has been addressed in versions 1.5.3 and 2.1.4. Users are advised to upgrade. There are no known workarounds for this vulnerability.
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-11-14T20:15Z
Repository: https://github.com/TYPO3/html-sanitizer
Language: PHP
Files changed: src/Behavior.php, src/Builder/CommonBuilder.php, src/Visitor/CommonVisitor.php, tests/CommonBuilderTest.php
Commit date: 2023-11-14 08:41:08 +0100
Commit message: [SECURITY] Deny processing instructions

XML processing instructions (e.g. `<?xml>`)
are denied during the sanitization process.

Diff: diff --git a/src/Behavior.php b/src/Behavior.php
index d26c10c..ae2b829 100644
--- a/src/Behavior.php
+++ b/src/Behavior.php
@@ -60,10 +60,15 @@ class Behavior
     /**
      * in case an unexpected CDATA section was found, encode the whole CDATA section as HTML
      */
     public const ENCODE_INVALID_CDATA_SECTION = 32;
 
+    /**
+     * in case an unexpected processing instruction (e.g. `<?xml>`) was found, encode the whole node as HTML
+     */
+    public const ENCODE_INVALID_PROCESSING_INSTRUCTION = 64;
+
     /**
      * @var int
      */
     protected $flags = 0;
 
@@ -222,10 +227,15 @@ class Behavior
     public function shallEncodeInvalidCdataSection(): bool
     {
         return ($this->flags & self::ENCODE_INVALID_CDATA_SECTION) === self::ENCODE_INVALID_CDATA_SECTION;
     }
 
+    public function shallEncodeInvalidProcessingInstruction(): bool
+    {
+        return ($this->flags & self::ENCODE_INVALID_PROCESSING_INSTRUCTION) === self::ENCODE_INVALID_PROCESSING_INSTRUCTION;
+    }
+
     public function shallRemoveUnexpectedChildren(): bool
     {
         return ($this->flags & self::REMOVE_UNEXPECTED_CHILDREN) === self::REMOVE_UNEXPECTED_CHILDREN;
     }
 
diff --git a/src/Builder/CommonBuilder.php b/src/Builder/CommonBuilder.php
index 9e38f08..71daf88 100644
--- a/src/Builder/CommonBuilder.php
+++ b/src/Builder/CommonBuilder.php
@@ -74,11 +74,15 @@ class CommonBuilder implements BuilderInterface
     }
 
     protected function createBehavior(): Behavior
     {
         return (new Behavior())
-            ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
+            ->withFlags(
+                Behavior::ENCODE_INVALID_TAG
+                | Behavior::REMOVE_UNEXPECTED_CHILDREN
+                | Behavior::ENCODE_INVALID_PROCESSING_INSTRUCTION
+            )
             ->withName('common')
             ->withTags(...array_values($this->createBasicTags()))
             ->withTags(...array_values($this->createMediaTags()))
             ->withTags(...array_values($this->createTableTags()));
     }
diff --git a/src/Visitor/CommonVisitor.php b/src/Visitor/CommonVisitor.php
index 64381e0..d92a042 100644
--- a/src/Visitor/CommonVisitor.php
+++ b/src/Visitor/CommonVisitor.php
@@ -17,10 +17,11 @@ namespace TYPO3\HtmlSanitizer\Visitor;
 use DOMAttr;
 use DOMCdataSection;
 use DOMComment;
 use DOMElement;
 use DOMNode;
+use DOMProcessingInstruction;
 use DOMText;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
 use Psr\Log\NullLogger;
 use TYPO3\HtmlSanitizer\Behavior;
@@ -62,10 +63,14 @@ class CommonVisitor extends AbstractVisitor implements LoggerAwareInterface
         }
     }
 
     public function enterNode(DOMNode $domNode): ?DOMNode
     {
+        if ($domNode instanceof DOMProcessingInstruction) {
+            return $this->handleInvalidNode($domNode);
+        }
+
         if (!$domNode instanceof DOMCdataSection
             && !$domNode instanceof DOMComment
             && !$domNode instanceof DOMElement
         ) {
             return $domNode;
@@ -217,10 +222,11 @@ class CommonVisitor extends AbstractVisitor implements LoggerAwareInterface
     protected function handleInvalidNode(DOMNode $domNode): ?DOMNode
     {
         if (
             ($domNode instanceof DOMComment && $this->behavior->shallEncodeInvalidComment())
             || ($domNode instanceof DOMCdataSection && $this->behavior->shallEncodeInvalidCdataSection())
+            || ($domNode instanceof DOMProcessingInstruction && $this->behavior->shallEncodeInvalidProcessingInstruction())
         ) {
             $this->log('Found unexpected node {nodeName}', [
                 'behavior' => $this->behavior->getName(),
                 'nodeType' => $domNode->nodeType,
                 'nodeName' => $domNode->nodeName,
diff --git a/tests/CommonBuilderTest.php b/tests/CommonBuilderTest.php
index a4ca310..2e2da3e 100644
--- a/tests/CommonBuilderTest.php
+++ b/tests/CommonBuilderTest.php
@@ -261,10 +261,18 @@ class CommonBuilderTest extends TestCase
             ],
             '#911' => [
                 '<!-- &lt;&quot;comment&quot;&gt; -->',
                 '<!-- &lt;&quot;comment&quot;&gt; -->',
             ],
+            '#912' => [
+                '<!---><p>',
+                '<!---&gt;&lt;p&gt;-->',
+            ],
+            '#913' => [
+                '<!---!><p>',
+                '<!---!&gt;&lt;p&gt;-->',
+            ],
             '#915' => [
                 '#text',
                 '#text',
             ],
             '#920' => [
@@ -301,10 +309,14 @@ class CommonBuilderTest extends TestCase
             ],
             '#936' => [
                 '<p class="{&quot;json&quot;:true}">value</p>',
                 '<p class="{&quot;json&quot;:true}">value</p>',
             ],
+            '#941' => [
+                '<?xml >s<img src=x onerror=alert(1)> ?>',
+                '&lt;?xml &gt;s&lt;img src=x onerror=alert(1)&gt; ?&gt;',
+            ],
         ];
     }
 
     /**
      * @param string $payload

Vulnerable code:             ->withFlags(Behavior::ENCODE_INVALID_TAG | Behavior::REMOVE_UNEXPECTED_CHILDREN)
Fixed code:     /**
     * in case an unexpected processing instruction (e.g. `<?xml>`) was found, encode the whole node as HTML
     */
    public const ENCODE_INVALID_PROCESSING_INSTRUCTION = 64;

    public function shallEncodeInvalidProcessingInstruction(): bool
    {
        return ($this->flags & self::ENCODE_INVALID_PROCESSING_INSTRUCTION) === self::ENCODE_INVALID_PROCESSING_INSTRUCTION;
    }

            ->withFlags(
                Behavior::ENCODE_INVALID_TAG
                | Behavior::REMOVE_UNEXPECTED_CHILDREN
                | Behavior::ENCODE_INVALID_PROCESSING_INSTRUCTION
            )
use DOMProcessingInstruction;
        if ($domNode instanceof DOMProcessingInstruction) {
            return $this->handleInvalidNode($domNode);
        }

            || ($domNode instanceof DOMProcessingInstruction && $this->behavior->shallEncodeInvalidProcessingInstruction())
            '#912' => [
                '<!---><p>',
                '<!---&gt;&lt;p&gt;-->',
            ],
            '#913' => [
                '<!---!><p>',
                '<!---!&gt;&lt;p&gt;-->',
            ],
            '#941' => [
                '<?xml >s<img src=x onerror=alert(1)> ?>',
                '&lt;?xml &gt;s&lt;img src=x onerror=alert(1)&gt; ?&gt;',
            ],