From 43b66331830713ddac84a5b61848e992edd064b6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 3 May 2021 23:59:52 +0100 Subject: [PATCH] Filtered scripts in custom HTML head for exports Since it appeared to cause problems in some scenarios. Related to #2490 --- app/Entities/Tools/PageContent.php | 64 +---------------- app/Http/Controllers/HomeController.php | 2 +- app/Util/HtmlContentFilter.php | 71 +++++++++++++++++++ resources/views/books/export.blade.php | 2 +- resources/views/chapters/export.blade.php | 2 +- resources/views/pages/export.blade.php | 2 +- .../partials/custom-head-content.blade.php | 5 -- .../views/partials/custom-head.blade.php | 6 +- .../partials/export-custom-head.blade.php | 5 ++ tests/Entity/ExportTest.php | 16 +++++ 10 files changed, 101 insertions(+), 74 deletions(-) create mode 100644 app/Util/HtmlContentFilter.php delete mode 100644 resources/views/partials/custom-head-content.blade.php create mode 100644 resources/views/partials/export-custom-head.blade.php diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 82499cdf2..ff502d164 100644 --- 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 31736e1b0..1ffb99f8d 100644 --- 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 index 000000000..cec927a3c --- /dev/null +++ b/app/Util/HtmlContentFilter.php @@ -0,0 +1,71 @@ +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 f62b89582..1faa3880e 100644 --- a/resources/views/books/export.blade.php +++ b/resources/views/books/export.blade.php @@ -27,7 +27,7 @@ } @yield('head') - @include('partials.custom-head') + @include('partials.export-custom-head') diff --git a/resources/views/chapters/export.blade.php b/resources/views/chapters/export.blade.php index 506e8db3d..96d9d7700 100644 --- a/resources/views/chapters/export.blade.php +++ b/resources/views/chapters/export.blade.php @@ -19,7 +19,7 @@ } } - @include('partials.custom-head') + @include('partials.export-custom-head') diff --git a/resources/views/pages/export.blade.php b/resources/views/pages/export.blade.php index 47a4d870a..1f2e60576 100644 --- a/resources/views/pages/export.blade.php +++ b/resources/views/pages/export.blade.php @@ -29,7 +29,7 @@ @endif - @include('partials.custom-head') + @include('partials.export-custom-head') diff --git a/resources/views/partials/custom-head-content.blade.php b/resources/views/partials/custom-head-content.blade.php deleted file mode 100644 index b245b7ad6..000000000 --- a/resources/views/partials/custom-head-content.blade.php +++ /dev/null @@ -1,5 +0,0 @@ -@if(setting('app-custom-head', false)) - - {!! setting('app-custom-head') !!} - -@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 dd7cc41e4..fa5ba0cc4 100644 --- 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') - - {!! setting('app-custom-head') !!} - + +{!! setting('app-custom-head') !!} + @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 index 000000000..f428e9fe9 --- /dev/null +++ b/resources/views/partials/export-custom-head.blade.php @@ -0,0 +1,5 @@ +@if(setting('app-custom-head')) + +{!! \BookStack\Util\HtmlContentFilter::removeScripts(setting('app-custom-head')) !!} + +@endif \ No newline at end of file diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php index 05672c6ca..d04ccc69a 100644 --- a/tests/Entity/ExportTest.php +++ b/tests/Entity/ExportTest.php @@ -1,5 +1,6 @@ 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', ''); + + 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; }'); + } + } + }