Filtered scripts in custom HTML head for exports

Since it appeared to cause problems in some scenarios.
Related to #2490

diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php
index 82499cdf2d28030f5930e5f4e94fff11f8a3728a..ff502d1640c5de2252d4d0c1acbce43255c87133 100644 (file)
--- a/app/Entities/Tools/PageContent.php
+++ b/app/Entities/Tools/PageContent.php
@@ -4,6 +4,7 @@ use BookStack\Entities\Models\Page;
 use BookStack\Entities\Tools\Markdown\CustomStrikeThroughExtension;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
+use BookStack\Util\HtmlContentFilter;
 use DOMDocument;
 use DOMNodeList;
 use DOMXPath;
@@ -169,7 +170,7 @@ class PageContent
         $content = $this->page->html;
 
         if (!config('app.allow_content_scripts')) {
-            $content = $this->escapeScripts($content);
+            $content = HtmlContentFilter::removeScripts($content);
         }
 
         if ($blankIncludes) {
@@ -308,65 +309,4 @@ class PageContent
 
         return $innerContent;
     }
-
-    /**
-     * Escape script tags within HTML content.
-     */
-    protected function escapeScripts(string $html) : string
-    {
-        if (empty($html)) {
-            return $html;
-        }
-
-        libxml_use_internal_errors(true);
-        $doc = new DOMDocument();
-        $doc->loadHTML(mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8'));
-        $xPath = new DOMXPath($doc);
-
-        // Remove standard script tags
-        $scriptElems = $xPath->query('//script');
-        foreach ($scriptElems as $scriptElem) {
-            $scriptElem->parentNode->removeChild($scriptElem);
-        }
-
-        // Remove clickable links to JavaScript URI
-        $badLinks = $xPath->query('//*[contains(@href, \'javascript:\')]');
-        foreach ($badLinks as $badLink) {
-            $badLink->parentNode->removeChild($badLink);
-        }
-
-        // Remove forms with calls to JavaScript URI
-        $badForms = $xPath->query('//*[contains(@action, \'javascript:\')] | //*[contains(@formaction, \'javascript:\')]');
-        foreach ($badForms as $badForm) {
-            $badForm->parentNode->removeChild($badForm);
-        }
-
-        // Remove meta tag to prevent external redirects
-        $metaTags = $xPath->query('//meta[contains(@content, \'url\')]');
-        foreach ($metaTags as $metaTag) {
-            $metaTag->parentNode->removeChild($metaTag);
-        }
-
-        // Remove data or JavaScript iFrames
-        $badIframes = $xPath->query('//*[contains(@src, \'data:\')] | //*[contains(@src, \'javascript:\')] | //*[@srcdoc]');
-        foreach ($badIframes as $badIframe) {
-            $badIframe->parentNode->removeChild($badIframe);
-        }
-
-        // Remove 'on*' attributes
-        $onAttributes = $xPath->query('//@*[starts-with(name(), \'on\')]');
-        foreach ($onAttributes as $attr) {
-            /** @var \DOMAttr $attr*/
-            $attrName = $attr->nodeName;
-            $attr->parentNode->removeAttribute($attrName);
-        }
-
-        $html = '';
-        $topElems = $doc->documentElement->childNodes->item(0)->childNodes;
-        foreach ($topElems as $child) {
-            $html .= $doc->saveHTML($child);
-        }
-
-        return $html;
-    }
 }

diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php
index 31736e1b09ab92bcb778c9d61dd928e3dbfa884a..1ffb99f8d427a00672683af60c35f6061e015705 100644 (file)
--- a/app/Http/Controllers/HomeController.php
+++ b/app/Http/Controllers/HomeController.php
@@ -105,7 +105,7 @@ class HomeController extends Controller
      */
     public function customHeadContent()
     {
-        return view('partials.custom-head-content');
+        return view('partials.custom-head');
     }
 
     /**

diff --git a/app/Util/HtmlContentFilter.php b/app/Util/HtmlContentFilter.php
new file mode 100644 (file)
index 0000000..cec927a
--- /dev/null
+++ b/app/Util/HtmlContentFilter.php
@@ -0,0 +1,71 @@
+<?php namespace BookStack\Util;
+
+use DOMDocument;
+use DOMNode;
+use DOMNodeList;
+use DOMXPath;
+
+class HtmlContentFilter
+{
+    /**
+     * Remove all of the script elements from the given HTML.
+     */
+    public static function removeScripts(string $html): string
+    {
+        if (empty($html)) {
+            return $html;
+        }
+
+        libxml_use_internal_errors(true);
+        $doc = new DOMDocument();
+        $doc->loadHTML(mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8'));
+        $xPath = new DOMXPath($doc);
+
+        // Remove standard script tags
+        $scriptElems = $xPath->query('//script');
+        static::removeNodes($scriptElems);
+
+        // Remove clickable links to JavaScript URI
+        $badLinks = $xPath->query('//*[contains(@href, \'javascript:\')]');
+        static::removeNodes($badLinks);
+
+        // Remove forms with calls to JavaScript URI
+        $badForms = $xPath->query('//*[contains(@action, \'javascript:\')] | //*[contains(@formaction, \'javascript:\')]');
+        static::removeNodes($badForms);
+
+        // Remove meta tag to prevent external redirects
+        $metaTags = $xPath->query('//meta[contains(@content, \'url\')]');
+        static::removeNodes($metaTags);
+
+        // Remove data or JavaScript iFrames
+        $badIframes = $xPath->query('//*[contains(@src, \'data:\')] | //*[contains(@src, \'javascript:\')] | //*[@srcdoc]');
+        static::removeNodes($badIframes);
+
+        // Remove 'on*' attributes
+        $onAttributes = $xPath->query('//@*[starts-with(name(), \'on\')]');
+        foreach ($onAttributes as $attr) {
+            /** @var \DOMAttr $attr*/
+            $attrName = $attr->nodeName;
+            $attr->parentNode->removeAttribute($attrName);
+        }
+
+        $html = '';
+        $topElems = $doc->documentElement->childNodes->item(0)->childNodes;
+        foreach ($topElems as $child) {
+            $html .= $doc->saveHTML($child);
+        }
+
+        return $html;
+    }
+
+    /**
+     * Removed all of the given DOMNodes.
+     */
+    static protected function removeNodes(DOMNodeList $nodes): void
+    {
+        foreach ($nodes as $node) {
+            $node->parentNode->removeChild($node);
+        }
+    }
+
+}
\ No newline at end of file

diff --git a/resources/views/books/export.blade.php b/resources/views/books/export.blade.php
index f62b895827b7c87b0a3f47ed82d66037fd1d5ad1..1faa3880e722c63cc51fc793ea7abdbfa28f2779 100644 (file)
--- a/resources/views/books/export.blade.php
+++ b/resources/views/books/export.blade.php
@@ -27,7 +27,7 @@
         }
     </style>
     @yield('head')
-    @include('partials.custom-head')
+    @include('partials.export-custom-head')
 </head>
 <body>
 

diff --git a/resources/views/chapters/export.blade.php b/resources/views/chapters/export.blade.php
index 506e8db3d40f7c18b5c7ca60fefb3ca4fe0c5ff6..96d9d77005754ee4fb483349562bb429ef8e24d6 100644 (file)
--- a/resources/views/chapters/export.blade.php
+++ b/resources/views/chapters/export.blade.php
@@ -19,7 +19,7 @@
             }
         }
     </style>
-    @include('partials.custom-head')
+    @include('partials.export-custom-head')
 </head>
 <body>
 

diff --git a/resources/views/pages/export.blade.php b/resources/views/pages/export.blade.php
index 47a4d870a041be8e10d4a11704e5b2bd9a4bb1d2..1f2e605768a5238e201f52b2c9157ed9be17bfbd 100644 (file)
--- a/resources/views/pages/export.blade.php
+++ b/resources/views/pages/export.blade.php
@@ -29,7 +29,7 @@
         </style>
     @endif
 
-    @include('partials.custom-head')
+    @include('partials.export-custom-head')
 </head>
 <body>
 

diff --git a/resources/views/partials/custom-head-content.blade.php b/resources/views/partials/custom-head-content.blade.php
deleted file mode 100644 (file)
index b245b7a..0000000
--- a/resources/views/partials/custom-head-content.blade.php
+++ /dev/null
@@ -1,5 +0,0 @@
-@if(setting('app-custom-head', false))
-    <!-- Custom user content -->
-    {!! setting('app-custom-head') !!}
-    <!-- End custom user content -->
-@endif
\ No newline at end of file

diff --git a/resources/views/partials/custom-head.blade.php b/resources/views/partials/custom-head.blade.php
index dd7cc41e43a2f81da99d4bef76362294c71c3d8b..fa5ba0cc456333776de144372098e68fc7f3fe65 100644 (file)
--- a/resources/views/partials/custom-head.blade.php
+++ b/resources/views/partials/custom-head.blade.php
@@ -1,5 +1,5 @@
 @if(setting('app-custom-head') && \Route::currentRouteName() !== 'settings')
-    <!-- Custom user content -->
-    {!! setting('app-custom-head') !!}
-    <!-- End custom user content -->
+<!-- Custom user content -->
+{!! setting('app-custom-head') !!}
+<!-- End custom user content -->
 @endif
\ No newline at end of file

diff --git a/resources/views/partials/export-custom-head.blade.php b/resources/views/partials/export-custom-head.blade.php
new file mode 100644 (file)
index 0000000..f428e9f
--- /dev/null
+++ b/resources/views/partials/export-custom-head.blade.php
@@ -0,0 +1,5 @@
+@if(setting('app-custom-head'))
+<!-- Custom user content -->
+{!! \BookStack\Util\HtmlContentFilter::removeScripts(setting('app-custom-head')) !!}
+<!-- End custom user content -->
+@endif
\ No newline at end of file

diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php
index 05672c6ca47b09252ea8ff87bf00b190c5a20b5c..d04ccc69a85304184e5c29293ec4644dcf6acf97 100644 (file)
--- a/tests/Entity/ExportTest.php
+++ b/tests/Entity/ExportTest.php
@@ -1,5 +1,6 @@
 <?php namespace Tests\Entity;
 
+use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use Illuminate\Support\Facades\Storage;
@@ -214,4 +215,19 @@ class ExportTest extends TestCase
         $resp->assertSee('src="/uploads/svg_test.svg"');
     }
 
+    public function test_exports_removes_scripts_from_custom_head()
+    {
+        $entities = [
+            Page::query()->first(), Chapter::query()->first(), Book::query()->first(),
+        ];
+        setting()->put('app-custom-head', '<script>window.donkey = "cat";</script><style>.my-test-class { color: red; }</style>');
+
+        foreach ($entities as $entity) {
+            $resp = $this->asEditor()->get($entity->getUrl('/export/html'));
+            $resp->assertDontSee('window.donkey');
+            $resp->assertDontSee('script');
+            $resp->assertSee('.my-test-class { color: red; }');
+        }
+    }
+
 }