diff --git a/app/Util/HtmlContentFilter.php b/app/Util/HtmlContentFilter.php index 729b80474..f3f29ae04 100644 --- a/app/Util/HtmlContentFilter.php +++ b/app/Util/HtmlContentFilter.php @@ -2,6 +2,7 @@ namespace BookStack\Util; +use DOMAttr; use DOMDocument; use DOMNodeList; use DOMXPath; @@ -43,13 +44,14 @@ class HtmlContentFilter $badIframes = $xPath->query('//*[' . static::xpathContains('@src', 'data:') . '] | //*[' . static::xpathContains('@src', 'javascript:') . '] | //*[@srcdoc]'); static::removeNodes($badIframes); + // Remove elements with a xlink:href attribute + // Used in SVG but deprecated anyway, so we'll be a bit more heavy-handed here. + $xlinkHrefAttributes = $xPath->query('//@*[contains(name(), \'xlink:href\')]'); + static::removeAttributes($xlinkHrefAttributes); + // Remove 'on*' attributes $onAttributes = $xPath->query('//@*[starts-with(name(), \'on\')]'); - foreach ($onAttributes as $attr) { - /** @var \DOMAttr $attr */ - $attrName = $attr->nodeName; - $attr->parentNode->removeAttribute($attrName); - } + static::removeAttributes($onAttributes); $html = ''; $topElems = $doc->documentElement->childNodes->item(0)->childNodes; @@ -72,7 +74,7 @@ class HtmlContentFilter } /** - * Removed all of the given DOMNodes. + * Remove all the given DOMNodes. */ protected static function removeNodes(DOMNodeList $nodes): void { @@ -80,4 +82,16 @@ class HtmlContentFilter $node->parentNode->removeChild($node); } } + + /** + * Remove all the given attribute nodes. + */ + protected static function removeAttributes(DOMNodeList $attrs): void + { + /** @var DOMAttr $attr */ + foreach ($attrs as $attr) { + $attrName = $attr->nodeName; + $attr->parentNode->removeAttribute($attrName); + } + } } diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 193f81400..1b2ce2db2 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -305,6 +305,28 @@ class PageContentTest extends TestCase $pageView->assertDontSee('abc123abc123'); } + public function test_svg_xlink_hrefs_are_removed() + { + $checks = [ + '', + '' + ]; + + $this->asEditor(); + $page = Page::query()->first(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertElementNotContains('.page-content', 'alert'); + $pageView->assertElementNotContains('.page-content', 'xlink:href'); + $pageView->assertElementNotContains('.page-content', 'application/xml'); + } + } + public function test_page_inline_on_attributes_show_if_configured() { $this->asEditor();