diff --git a/app/Entities/Repos/EntityRepo.php b/app/Entities/Repos/EntityRepo.php index 1cff46da1..a0934530f 100644 --- a/app/Entities/Repos/EntityRepo.php +++ b/app/Entities/Repos/EntityRepo.php @@ -1,5 +1,6 @@ .*?<\/script>/ms'; - $matches = []; - preg_match_all($scriptSearchRegex, $html, $matches); - - foreach ($matches[0] as $match) { - $html = str_replace($match, htmlentities($match), $html); + if ($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('//body//*//script'); + foreach ($scriptElems as $scriptElem) { + $scriptElem->parentNode->removeChild($scriptElem); + } + + // Remove 'on*' attributes + $onAttributes = $xPath->query('//body//*/@*[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; } @@ -773,8 +800,8 @@ class EntityRepo /** * Destroy a bookshelf instance - * @param \BookStack\Entities\Bookshelf $shelf - * @throws \Throwable + * @param Bookshelf $shelf + * @throws Throwable */ public function destroyBookshelf(Bookshelf $shelf) { @@ -784,9 +811,9 @@ class EntityRepo /** * Destroy the provided book and all its child entities. - * @param \BookStack\Entities\Book $book + * @param Book $book * @throws NotifyException - * @throws \Throwable + * @throws Throwable */ public function destroyBook(Book $book) { @@ -802,8 +829,8 @@ class EntityRepo /** * Destroy a chapter and its relations. - * @param \BookStack\Entities\Chapter $chapter - * @throws \Throwable + * @param Chapter $chapter + * @throws Throwable */ public function destroyChapter(Chapter $chapter) { @@ -821,7 +848,7 @@ class EntityRepo * Destroy a given page along with its dependencies. * @param Page $page * @throws NotifyException - * @throws \Throwable + * @throws Throwable */ public function destroyPage(Page $page) { @@ -844,12 +871,12 @@ class EntityRepo /** * Destroy or handle the common relations connected to an entity. - * @param \BookStack\Entities\Entity $entity - * @throws \Throwable + * @param Entity $entity + * @throws Throwable */ protected function destroyEntityCommonRelations(Entity $entity) { - \Activity::removeEntity($entity); + Activity::removeEntity($entity); $entity->views()->delete(); $entity->permissions()->delete(); $entity->tags()->delete(); @@ -861,9 +888,9 @@ class EntityRepo /** * Copy the permissions of a bookshelf to all child books. * Returns the number of books that had permissions updated. - * @param \BookStack\Entities\Bookshelf $bookshelf + * @param Bookshelf $bookshelf * @return int - * @throws \Throwable + * @throws Throwable */ public function copyBookshelfPermissions(Bookshelf $bookshelf) { diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 88169c50d..6201cf5d7 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -71,17 +71,30 @@ class PageContentTest extends TestCase $pageResp->assertSee($content); } - public function test_page_content_scripts_escaped_by_default() + public function test_page_content_scripts_removed_by_default() { $this->asEditor(); $page = Page::first(); - $script = ''; + $script = 'abc123abc123'; $page->html = "escape {$script}"; $page->save(); $pageView = $this->get($page->getUrl()); $pageView->assertDontSee($script); - $pageView->assertSee(htmlentities($script)); + $pageView->assertSee('abc123abc123'); + } + + public function test_page_inline_on_attributes_removed_by_default() + { + $this->asEditor(); + $page = Page::first(); + $script = '

Hello

'; + $page->html = "escape {$script}"; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertDontSee($script); + $pageView->assertSee('

Hello

'); } public function test_page_content_scripts_show_when_configured() @@ -89,13 +102,29 @@ class PageContentTest extends TestCase $this->asEditor(); $page = Page::first(); config()->push('app.allow_content_scripts', 'true'); - $script = ''; + + $script = 'abc123abc123'; $page->html = "no escape {$script}"; $page->save(); $pageView = $this->get($page->getUrl()); $pageView->assertSee($script); - $pageView->assertDontSee(htmlentities($script)); + $pageView->assertDontSee('abc123abc123'); + } + + public function test_page_inline_on_attributes_show_if_configured() + { + $this->asEditor(); + $page = Page::first(); + config()->push('app.allow_content_scripts', 'true'); + + $script = '

Hello

'; + $page->html = "escape {$script}"; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertSee($script); + $pageView->assertDontSee('

Hello

'); } public function test_duplicate_ids_does_not_break_page_render()