From: Dan Brown <redacted>
Date: Thu, 2 Sep 2021 21:02:30 +0000 (+0100)
Subject: Added extra HTML filtering of dangerous content
X-Git-Tag: v21.08.2~1^2^2~4
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/5e6092aaf8fd420202016038286554860bf8ea64

Added extra HTML filtering of dangerous content

In particular, That around the casing of dangerous values within
attributes. This uses some xpath translation to handle different casing
in contains searching.
---

diff --git a/app/Util/HtmlContentFilter.php b/app/Util/HtmlContentFilter.php
index f251a22fd..729b80474 100644
--- a/app/Util/HtmlContentFilter.php
+++ b/app/Util/HtmlContentFilter.php
@@ -28,19 +28,19 @@ class HtmlContentFilter
         static::removeNodes($scriptElems);
 
         // Remove clickable links to JavaScript URI
-        $badLinks = $xPath->query('//*[contains(@href, \'javascript:\')]');
+        $badLinks = $xPath->query('//*[' . static::xpathContains('@href', 'javascript:') . ']');
         static::removeNodes($badLinks);
 
         // Remove forms with calls to JavaScript URI
-        $badForms = $xPath->query('//*[contains(@action, \'javascript:\')] | //*[contains(@formaction, \'javascript:\')]');
+        $badForms = $xPath->query('//*[' . static::xpathContains('@action', 'javascript:') . '] | //*[' . static::xpathContains('@formaction', 'javascript:') . ']');
         static::removeNodes($badForms);
 
         // Remove meta tag to prevent external redirects
-        $metaTags = $xPath->query('//meta[contains(@content, \'url\')]');
+        $metaTags = $xPath->query('//meta[' . static::xpathContains('@content', 'url') . ']');
         static::removeNodes($metaTags);
 
         // Remove data or JavaScript iFrames
-        $badIframes = $xPath->query('//*[contains(@src, \'data:\')] | //*[contains(@src, \'javascript:\')] | //*[@srcdoc]');
+        $badIframes = $xPath->query('//*[' . static::xpathContains('@src', 'data:') . '] | //*[' . static::xpathContains('@src', 'javascript:') . '] | //*[@srcdoc]');
         static::removeNodes($badIframes);
 
         // Remove 'on*' attributes
@@ -60,6 +60,17 @@ class HtmlContentFilter
         return $html;
     }
 
+    /**
+     * Create a xpath contains statement with a translation automatically built within
+     * to affectively search in a cases-insensitive manner.
+     */
+    protected static function xpathContains(string $property, string $value): string
+    {
+        $value = strtolower($value);
+        $upperVal = strtoupper($value);
+        return 'contains(translate(' . $property . ', \'' . $upperVal . '\', \'' . $value . '\'), \'' . $value . '\')';
+    }
+
     /**
      * Removed all of the given DOMNodes.
      */
diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php
index 5aee97887..193f81400 100644
--- a/tests/Entity/PageContentTest.php
+++ b/tests/Entity/PageContentTest.php
@@ -135,14 +135,26 @@ class PageContentTest extends TestCase
         }
     }
 
-    public function test_iframe_js_and_base64_urls_are_removed()
+    public function test_js_and_base64_src_urls_are_removed()
     {
         $checks = [
             '<iframe src="javascript:alert(document.cookie)"></iframe>',
+            '<iframe src="JavAScRipT:alert(document.cookie)"></iframe>',
+            '<iframe src="JavAScRipT:alert(document.cookie)"></iframe>',
             '<iframe SRC=" javascript: alert(document.cookie)"></iframe>',
             '<iframe src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
+            '<iframe src="DaTa:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
             '<iframe src=" data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg==" frameborder="0"></iframe>',
+            '<img src="javascript:alert(document.cookie)"/>',
+            '<img src="JavAScRipT:alert(document.cookie)"/>',
+            '<img src="JavAScRipT:alert(document.cookie)"/>',
+            '<img SRC=" javascript: alert(document.cookie)"/>',
+            '<img src="data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
+            '<img src="DaTa:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
+            '<img src=" data:text/html;base64,PHNjcmlwdD5hbGVydCgnaGVsbG8nKTwvc2NyaXB0Pg=="/>',
             '<iframe srcdoc="<script>window.alert(document.cookie)</script>"></iframe>',
+            '<iframe SRCdoc="<script>window.alert(document.cookie)</script>"></iframe>',
+            '<IMG SRC=`javascript:alert("RSnake says, \'XSS\'")`>',
         ];
 
         $this->asEditor();
@@ -155,6 +167,7 @@ class PageContentTest extends TestCase
             $pageView = $this->get($page->getUrl());
             $pageView->assertStatus(200);
             $pageView->assertElementNotContains('.page-content', '<iframe>');
+            $pageView->assertElementNotContains('.page-content', '<img');
             $pageView->assertElementNotContains('.page-content', '</iframe>');
             $pageView->assertElementNotContains('.page-content', 'src=');
             $pageView->assertElementNotContains('.page-content', 'javascript:');
@@ -168,6 +181,8 @@ class PageContentTest extends TestCase
         $checks = [
             '<a id="xss" href="javascript:alert(document.cookie)>Click me</a>',
             '<a id="xss" href="javascript: alert(document.cookie)>Click me</a>',
+            '<a id="xss" href="JaVaScRiPt: alert(document.cookie)>Click me</a>',
+            '<a id="xss" href=" JaVaScRiPt: alert(document.cookie)>Click me</a>',
         ];
 
         $this->asEditor();
@@ -179,7 +194,7 @@ class PageContentTest extends TestCase
 
             $pageView = $this->get($page->getUrl());
             $pageView->assertStatus(200);
-            $pageView->assertElementNotContains('.page-content', '<a id="xss">');
+            $pageView->assertElementNotContains('.page-content', '<a id="xss"');
             $pageView->assertElementNotContains('.page-content', 'href=javascript:');
         }
     }
@@ -188,8 +203,10 @@ class PageContentTest extends TestCase
     {
         $checks = [
             '<form><input id="xss" type=submit formaction=javascript:alert(document.domain) value=Submit><input></form>',
+            '<form ><button id="xss" formaction="JaVaScRiPt:alert(document.domain)">Click me</button></form>',
             '<form ><button id="xss" formaction=javascript:alert(document.domain)>Click me</button></form>',
             '<form id="xss" action=javascript:alert(document.domain)><input type=submit value=Submit></form>',
+            '<form id="xss" action="JaVaScRiPt:alert(document.domain)"><input type=submit value=Submit></form>',
         ];
 
         $this->asEditor();
@@ -213,6 +230,8 @@ class PageContentTest extends TestCase
     {
         $checks = [
             '<meta http-equiv="refresh" content="0; url=//external_url">',
+            '<meta http-equiv="refresh" ConTeNt="0; url=//external_url">',
+            '<meta http-equiv="refresh" content="0; UrL=//external_url">',
         ];
 
         $this->asEditor();
@@ -249,11 +268,13 @@ class PageContentTest extends TestCase
     {
         $checks = [
             '<p onclick="console.log(\'test\')">Hello</p>',
+            '<p OnCliCk="console.log(\'test\')">Hello</p>',
             '<div>Lorem ipsum dolor sit amet.</div><p onclick="console.log(\'test\')">Hello</p>',
             '<div>Lorem ipsum dolor sit amet.<p onclick="console.log(\'test\')">Hello</p></div>',
             '<div><div><div><div>Lorem ipsum dolor sit amet.<p onclick="console.log(\'test\')">Hello</p></div></div></div></div>',
             '<div onclick="console.log(\'test\')">Lorem ipsum dolor sit amet.</div><p onclick="console.log(\'test\')">Hello</p><div></div>',
             '<a a="<img src=1 onerror=\'alert(1)\'> ',
+            '\<a onclick="alert(document.cookie)"\>xss link\</a\>',
         ];
 
         $this->asEditor();