From d162fab123fbebcac57602d4dd498a747abfbddb Mon Sep 17 00:00:00 2001
From: Steve Boyd <emteknetnz@gmail.com>
Date: Tue, 23 Aug 2022 16:44:58 +1200
Subject: [PATCH] [CVE-2022-37421] Sanitise ExtraMeta field for XSS

---
 code/Model/SiteTree.php          | 34 +++++++++++++++++++++++
 lang/en.yml                      |  1 +
 tests/php/Model/SiteTreeTest.php | 47 ++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php
index 2c19f445..7b2febcc 100755
--- a/code/Model/SiteTree.php
+++ b/code/Model/SiteTree.php
@@ -69,6 +69,7 @@ use SilverStripe\Versioned\RecursivePublishable;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\HTML;
+use SilverStripe\View\Parsers\HTMLValue;
 use SilverStripe\View\Parsers\ShortcodeParser;
 use SilverStripe\View\Parsers\URLSegmentFilter;
 use SilverStripe\View\Shortcodes\EmbedShortcodeProvider;
@@ -1684,6 +1685,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
             $this->setNextWriteWithoutVersion(true);
         }
 
+        $this->sanitiseExtraMeta();
+
         // Flush cached [embed] shortcodes
         // Flush on both DRAFT and LIVE because VersionedCacheAdapter has separate caches for both
         // Clear both caches at once for the scenario where a CMS-author updates a remote resource
@@ -1703,6 +1706,27 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
         }
     }
 
+    private function sanitiseExtraMeta(): void
+    {
+        $htmlValue = HTMLValue::create($this->ExtraMeta);
+        /** @var DOMElement $el */
+        foreach ($htmlValue->query('//*') as $el) {
+            /** @var DOMAttr $attr */
+            $attributes = $el->attributes;
+            for ($i = count($attributes) - 1; $i >= 0; $i--) {
+                $attr = $attributes->item($i);
+                // remove any attribute starting with 'on' e.g. onclick
+                // and remove the accesskey attribute
+                if (substr($attr->name, 0, 2) === 'on' ||
+                    $attr->name === 'accesskey'
+                ) {
+                    $el->removeAttributeNode($attr);
+                }
+            }
+        }
+        $this->ExtraMeta = $htmlValue->getContent();
+    }
+
     /**
      * Trigger synchronisation of link tracking
      *
@@ -1797,6 +1821,16 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
             );
         }
 
+        // Ensure ExtraMeta can be turned into valid HTML
+        if ($this->ExtraMeta && !HTMLValue::create($this->ExtraMeta)->getContent()) {
+            $result->addError(
+                _t(
+                    'SilverStripe\\CMS\\Model\\SiteTree.InvalidExtraMeta',
+                    'Custom Meta Tags does not contain valid HTML',
+                )
+            );
+        }
+
         return $result;
     }
 
diff --git a/lang/en.yml b/lang/en.yml
index a539d294..c66e1b4e 100644
--- a/lang/en.yml
+++ b/lang/en.yml
@@ -208,6 +208,7 @@ en:
     HTMLEDITORTITLE: Content
     INHERIT: 'Inherit from parent page'
     INHERITSITECONFIG: 'Inherit from site access settings'
+    InvalidExtraMeta: 'Custom Meta Tags does not contain valid HTML'
     LASTPUBLISHED: 'Last published'
     LASTSAVED: 'Last saved'
     LASTUPDATED: 'Last Updated'
diff --git a/tests/php/Model/SiteTreeTest.php b/tests/php/Model/SiteTreeTest.php
index d919a674..fb76748c 100644
--- a/tests/php/Model/SiteTreeTest.php
+++ b/tests/php/Model/SiteTreeTest.php
@@ -1986,4 +1986,51 @@ class SiteTreeTest extends SapphireTest
         );
         // END ARCHIVED
     }
+
+    /**
+     * @dataProvider provideSanitiseExtraMeta
+     */
+    public function testSanitiseExtraMeta(string $extraMeta, string $expected, string $message): void
+    {
+        $siteTree = new SiteTree();
+        $siteTree->ExtraMeta = $extraMeta;
+        $siteTree->write();
+        $this->assertSame($expected, $siteTree->ExtraMeta, $message);
+    }
+
+    public function provideSanitiseExtraMeta(): array
+    {
+        return [
+            [
+                '<link rel="canonical" accesskey="X" sometrigger="alert(1)" />',
+                '<link rel="canonical" sometrigger="alert(1)">',
+                'accesskey attribute is removed'
+            ],
+            [
+                '<link rel="canonical" onclick="alert(1)" /><meta name="x" onerror="alert(0)">',
+                '<link rel="canonical"><meta name="x">',
+                'Attributes starting with "on" are removed'
+            ],
+            [
+                '<link rel="canonical" onclick=alert(1) /><meta name="x" onerror=\'alert(0)\'>',
+                '<link rel="canonical"><meta name="x">',
+                'Attributes with different quote styles are removed'
+            ],
+            [
+                '<link rel="canonical" ONCLICK=alert(1) /><meta name="x" oNeRrOr=\'alert(0)\'>',
+                '<link rel="canonical"><meta name="x">',
+                'Mixed case attributes are removed'
+            ],
+            [
+                '<link rel="canonical" accesskey="X" onclick="alert(1)" name="x" />',
+                '<link rel="canonical" name="x">',
+                'Multiple attributes are removed'
+            ],
+            [
+                '<link rel="canonical" href="valid" ;;// somethingdodgy < onmouseover=alert(1)',
+                '<link rel="canonical" href="valid" somethingdodgy="">',
+                'Invalid HTML is converted to valid HTML and parsed'
+            ],
+        ];
+    }
 }