Updated version and assets for release v25.12.9

As suggested by Alex Dan in their security report.

app/Entities/Controllers/PageRevisionController.php

@@ -12,6 +12,8 @@ use BookStack\Exceptions\NotFoundException;
 use BookStack\Facades\Activity;
 use BookStack\Http\Controller;
 use BookStack\Permissions\Permission;
+use BookStack\Util\HtmlContentFilter;
+use BookStack\Util\HtmlContentFilterConfig;
 use BookStack\Util\SimpleListOptions;
 use Illuminate\Http\Request;
 use Ssddanbrown\HtmlDiff\Diff;
@@ -101,12 +103,15 @@ class PageRevisionController extends Controller
 
         $prev = $revision->getPreviousRevision();
         $prevContent = $prev->html ?? '';
-        $diff = Diff::excecute($prevContent, $revision->html);
+
+        // TODO - Refactor PageContent so we can de-dupe these steps
+        $rawDiff = Diff::excecute($prevContent, $revision->html);
+        $filterConfig = HtmlContentFilterConfig::fromConfigString(config('app.content_filtering'));
+        $filter = new HtmlContentFilter($filterConfig);
+        $diff = $filter->filterString($rawDiff);
 
         $page->fill($revision->toArray());
-        // TODO - Refactor PageContent so we don't need to juggle this
-        $page->html = $revision->html;
-        $page->html = (new PageContent($page))->render();
+        $page->html = '';
         $this->setPageTitle(trans('entities.pages_revision_named', ['pageName' => $page->getShortName()]));
 
         return view('pages.revision', [

app/Http/Controller.php

@@ -167,14 +167,26 @@ abstract class Controller extends BaseController
 
     /**
      * Redirect to the URL provided in the request as a '_return' parameter.
-     * Will check that the parameter leads to a URL under the root path of the system.
+     * Will check that the parameter leads to a URL under the same origin as the application.
      */
     protected function redirectToRequest(Request $request): RedirectResponse
     {
         $basePath = url('/');
         $returnUrl = $request->input('_return') ?? $basePath;
 
-        if (!str_starts_with($returnUrl, $basePath)) {
+        // Only allow use of _return on requests where we expect CSRF to be active
+        // to prevent it potentially being used as an open redirect
+        $allowedMethods = ['POST', 'PUT', 'PATCH', 'DELETE'];
+        if (!in_array($request->getMethod(), $allowedMethods)) {
+            return redirect($basePath);
+        }
+
+        $intendedUrl = parse_url($returnUrl);
+        $baseUrl = parse_url($basePath);
+        $isSameOrigin = ($intendedUrl['host'] ?? '') === ($baseUrl['host'] ?? '')
+            && ($intendedUrl['scheme'] ?? '') === ($baseUrl['scheme'] ?? '')
+            && ($intendedUrl['port'] ?? 0) === ($baseUrl['port'] ?? 0);
+        if (!$isSameOrigin) {
             return redirect($basePath);
         }
 

app/Util/ConfiguredHtmlPurifier.php

@@ -22,8 +22,13 @@ class ConfiguredHtmlPurifier
 
     public function __construct()
     {
+        // This is done by the web-server at run-time, with the existing
+        // storage/framework/cache folder to ensure we're using a server-writable folder.
+        $cachePath = storage_path('framework/cache/purifier');
+        $this->createCacheFolderIfNeeded($cachePath);
+
         $config = HTMLPurifier_HTML5Config::createDefault();
-        $this->setConfig($config);
+        $this->setConfig($config, $cachePath);
         $this->resetCacheIfNeeded($config);
 
         $htmlDef = $config->getDefinition('HTML', true, true);
@@ -34,6 +39,13 @@ class ConfiguredHtmlPurifier
         $this->purifier = new HTMLPurifier($config);
     }
 
+    protected function createCacheFolderIfNeeded(string $cachePath): void
+    {
+        if (!file_exists($cachePath)) {
+            mkdir($cachePath, 0777, true);
+        }
+    }
+
     protected function resetCacheIfNeeded(HTMLPurifier_Config $config): void
     {
         if (self::$cachedChecked) {
@@ -53,12 +65,14 @@ class ConfiguredHtmlPurifier
         self::$cachedChecked = true;
     }
 
-    protected function setConfig(HTMLPurifier_Config $config): void
+    protected function setConfig(HTMLPurifier_Config $config, string $cachePath): void
     {
-        $config->set('Cache.SerializerPath', storage_path('framework/purifier'));
+        $config->set('Cache.SerializerPath', $cachePath);
         $config->set('Core.AllowHostnameUnderscore', true);
         $config->set('CSS.AllowTricky', true);
         $config->set('HTML.SafeIframe', true);
+        $config->set('HTML.TargetNoopener', false);
+        $config->set('HTML.TargetNoreferrer', false);
         $config->set('Attr.EnableID', true);
         $config->set('Attr.ID.HTML5', true);
         $config->set('Output.FixInnerHTML', false);
@@ -122,6 +136,19 @@ class ConfiguredHtmlPurifier
                 'value' => 'Text',
             ]
         );
+
+        // Allow the drawio-diagram attribute on div elements
+        $definition->addAttribute(
+            'div',
+            'drawio-diagram',
+            'Number',
+        );
+
+        // Allow target="_blank" on links
+        $definition->addAttribute('a', 'target', 'Enum#_blank');
+
+        // Allow mention-ids on links
+        $definition->addAttribute('a', 'data-mention-user-id', 'Number');
     }
 
     public function purify(string $html): string

app/Util/HtmlDocument.php

@@ -103,7 +103,13 @@ class HtmlDocument
      */
     public function getBody(): DOMNode
     {
-        return $this->document->getElementsByTagName('body')[0];
+        $bodies = $this->document->getElementsByTagName('body');
+
+        if ($bodies->length === 0) {
+            return new DOMElement('body', '');
+        }
+
+        return $bodies[0];
     }
 
     /**

dev/checksums/vendor

@@ -1 +1 @@
-5732efe93a37a665ec9e526d713293b438e610dcf0c6e950fa7317907e480252
+e2acb4ea1edce1e2c9af0d7598a3de9ae1d44ff9e647f018f4f732ab5f560f9d

storage/framework/.gitignore

@@ -7,3 +7,4 @@ routes.php
 routes.scanned.php
 schedule-*
 services.json
+purifier/

storage/framework/purifier/.gitignore

@@ -1,2 +0,0 @@
-*
-!.gitignore

tests/Entity/PageContentFilteringTest.php

@@ -463,6 +463,7 @@ HTML;
             '<div style="position:absolute;left:0;color:#00FFEE;">Hello!</div>' => '<div style="color:#00FFEE;">Hello!</div>',
             '<div style="background:#FF0000;left:0;color:#00FFEE;">Hello!</div>' => '<div style="background:#FF0000;color:#00FFEE;">Hello!</div>',
             '<div style="color:#00FFEE;">Hello!<style>testinghello!</style></div>' => '<div style="color:#00FFEE;">Hello!</div>',
+            '<div drawio-diagram="5332" another-attr="cat">Hello!</div>' => '<div drawio-diagram="5332">Hello!</div>',
         ];
 
         config()->set('app.content_filtering', 'a');
@@ -477,4 +478,25 @@ HTML;
             $resp->assertSee($expected, false);
         }
     }
+
+    public function test_allow_list_does_not_filter_cases()
+    {
+        $testCasesExpectedByInput = [
+            '<p><a href="https://example.com" target="_blank">New tab linkydoodle</a></p>',
+            '<p><a href="https://example.com/user/1" data-mention-user-id="5">@mentionusertext</a></p>',
+            '<details><summary>Hello</summary><p>Mydetailshere</p></details>',
+        ];
+
+        config()->set('app.content_filtering', 'a');
+        $page = $this->entities->page();
+        $this->asEditor();
+
+        foreach ($testCasesExpectedByInput as $input) {
+            $page->html = $input;
+            $page->save();
+            $resp = $this->get($page->getUrl());
+
+            $resp->assertSee($input, false);
+        }
+    }
 }

tests/Entity/PageEditorTest.php

@@ -282,4 +282,23 @@ class PageEditorTest extends TestCase
         $resp->assertOk();
         $resp->assertDontSee('hellotherethisisaturtlemonster', false);
     }
+
+    public function test_editor_html_filtered_does_not_cause_error_if_empty()
+    {
+        $emptyExamples = ['', '<p></p>', '<p>&nbsp;</p>', ' ', "\n"];
+        $editor = $this->users->editor();
+        $page = $this->entities->page();
+        $page->updated_by = $editor->id;
+
+        foreach ($emptyExamples as $emptyExample) {
+            $page->html = $emptyExample;
+            $page->save();
+
+            $resp = $this->asAdmin()->get($page->getUrl('edit'));
+            $resp->assertOk();
+
+            $resp = $this->asAdmin()->get("/ajax/page/{$page->id}");
+            $resp->assertOk();
+        }
+    }
 }

tests/Entity/PageRevisionTest.php

@@ -47,6 +47,20 @@ class PageRevisionTest extends TestCase
         $revisionView->assertSee('new revision content');
     }
 
+    public function test_page_revision_preview_filters_html_content()
+    {
+        $this->asEditor();
+        $page = $this->entities->page();
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => '<script>dontwantthishere</script><style>dontwantthishere</style><p>expectthisthough</p>']);
+        $pageRevision = $page->revisions->last();
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => '<p>Updated content</p>']);
+
+        $revisionView = $this->get($page->getUrl() . '/revisions/' . $pageRevision->id);
+        $revisionView->assertStatus(200);
+        $revisionView->assertSee('expectthisthough');
+        $revisionView->assertDontSee('dontwantthishere');
+    }
+
     public function test_page_revision_restore_updates_content()
     {
         $this->asEditor();
@@ -215,6 +229,34 @@ class PageRevisionTest extends TestCase
         $html->assertElementContains('.item-list > .item-list-row:nth-child(2)', 'Changes');
     }
 
+    public function test_revision_changes_view_shows_diff()
+    {
+        $this->asEditor();
+        $page = $this->entities->page();
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => '<p id="bkmrk-hello">Hello there dog</p>']);
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => '<p id="bkmrk-hello">Hello there cat</p>']);
+
+        $pageRevision = $page->revisions()->orderBy('id', 'desc')->first();
+        $revisionView = $this->get("{$page->getUrl()}/revisions/{$pageRevision->id}/changes");
+        $revisionView->assertStatus(200);
+        $revisionView->assertSee('<p id="bkmrk-hello">Hello there <del class="diffmod">dog</del><ins class="diffmod">cat</ins></p>', false);
+    }
+
+    public function test_revision_changes_view_filters_html_content()
+    {
+        $this->asEditor();
+        $page = $this->entities->page();
+        $html = '<script>dontwantthishere</script><style>dontwantthishere</style><p>expectthisthough</p>';
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => $html]);
+        $this->createRevisions($page, 1, ['name' => 'updated page', 'html' => $html]);
+
+        $pageRevision = $page->revisions()->orderBy('id', 'desc')->first();
+        $revisionView = $this->get("{$page->getUrl()}/revisions/{$pageRevision->id}/changes");
+        $revisionView->assertStatus(200);
+        $revisionView->assertSee('expectthisthough');
+        $revisionView->assertDontSee('dontwantthishere');
+    }
+
     public function test_revision_restore_action_only_visible_with_permission()
     {
         $page = $this->entities->page();

tests/User/UserPreferencesTest.php

@@ -153,6 +153,26 @@ class UserPreferencesTest extends TestCase
             ->assertElementNotExists('.content-wrap .entity-list-item');
     }
 
+    public function test_redirect_on_preference_change_checks_host()
+    {
+        $expectedByRedirect = [
+            'http://localhost/beans' => 'http://localhost/beans',
+            'https://localhost/beans' => 'http://localhost',
+            'http://localhost:9090/beans' => 'http://localhost',
+            'http://localhost.example.com/beans' => 'http://localhost',
+            'http://localhost@example.com/beans' => 'http://localhost',
+        ];
+
+        $this->asEditor();
+        foreach ($expectedByRedirect as $url => $expected) {
+            $req = $this->patch("/preferences/change-view/bookshelf", [
+                'view' => 'grid',
+                '_return' => $url,
+            ]);
+            $req->assertRedirect($expected);
+        }
+    }
+
     public function test_update_code_language_favourite()
     {
         $editor = $this->users->editor();

version

@@ -1 +1 @@
-v25.12.4
+v25.12.9