]> BookStack Code Mirror - bookstack/commitdiff
Altered the parsing of custom head to prevent htmlentities on content
authorDan Brown <redacted>
Sun, 12 Sep 2021 15:19:17 +0000 (16:19 +0100)
committerDan Brown <redacted>
Sun, 12 Sep 2021 15:19:17 +0000 (16:19 +0100)
Was causing things like emjoi within script content to be somewhat
mangled. Instead we force UTF8 only parsing via XML declaration.

Added test to cover.

For #2923

app/Util/CspService.php
app/Util/HtmlNonceApplicator.php
tests/Settings/CustomHeadContentTest.php

index ec5021371d7b5b998c8ac43087beef091841a5e0..812e1a4bed1cc2a1d937028e2f614ccf89cfa5ba 100644 (file)
@@ -12,7 +12,7 @@ class CspService
 
     public function __construct(string $nonce = '')
     {
-        $this->nonce = $nonce ?: Str::random(16);
+        $this->nonce = $nonce ?: Str::random(24);
     }
 
     /**
index 2653b7075477030d2291e5eb191b568b7f6c5ca4..07298577cf564f4cd4f39b625cddd560770e64aa 100644 (file)
@@ -21,10 +21,10 @@ class HtmlNonceApplicator
             return $html;
         }
 
-        $html = '<body>' . $html . '</body>';
+        $html = '<?xml encoding="utf-8" ?><body>' . $html . '</body>';
         libxml_use_internal_errors(true);
         $doc = new DOMDocument();
-        $doc->loadHTML(mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8'), LIBXML_SCHEMA_CREATE);
+        $doc->loadHTML($html, LIBXML_SCHEMA_CREATE);
         $xPath = new DOMXPath($doc);
 
         // Apply to scripts
index 59d5fc06ccbe8b1ddac6cff8b98aace243de641b..94ef4711d5d640054ee0470516d3fccb3ad5eeaa 100644 (file)
@@ -2,6 +2,7 @@
 
 namespace Tests\Settings;
 
+use BookStack\Util\CspService;
 use Tests\TestCase;
 
 class CustomHeadContentTest extends TestCase
@@ -26,4 +27,40 @@ class CustomHeadContentTest extends TestCase
         $resp = $this->get('/login');
         $resp->assertSee('<div id="hello">cat</div>');
     }
+
+    public function test_nonce_application_handles_edge_cases()
+    {
+        $mockCSP = $this->mock(CspService::class);
+        $mockCSP->shouldReceive('getNonce')->andReturn('abc123');
+
+        $content = trim('
+<script>console.log("cat");</script>
+<script type="text/html"><\script>const a = `<div></div>`<\/\script></script>
+<script >const a = `<div></div>`;</script>
+<script type="<script text>test">const c = `<div></div>`;</script>
+<script
+    type="text/html"
+>
+const a = `<\script><\/script>`;
+const b = `<script`;
+</script>
+<SCRIPT>const b = `↗️£`;</SCRIPT>
+        ');
+
+        $expectedOutput = trim('
+<script nonce="abc123">console.log("cat");</script>
+<script type="text/html" nonce="abc123"><\script>const a = `<div></div>`<\/\script></script>
+<script nonce="abc123">const a = `<div></div>`;</script>
+<script type="&lt;script text&gt;test" nonce="abc123">const c = `<div></div>`;</script>
+<script type="text/html" nonce="abc123">
+const a = `<\script><\/script>`;
+const b = `<script`;
+</script>
+<script nonce="abc123">const b = `↗️£`;</script>
+        ');
+
+        $this->setSettings(['app-custom-head' => $content]);
+        $resp = $this->get('/login');
+        $resp->assertSee($expectedOutput);
+    }
 }