diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 436c4f0be..f8a061739 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -339,7 +339,7 @@ class PageContent { $contentHash = md5($html); $contentId = $this->page->id; - $contentTime = $this->page->updated_at->timestamp; + $contentTime = $this->page->updated_at?->timestamp ?? time(); $appVersion = AppVersion::get(); return "page-content-cache::{$appVersion}::{$contentId}::{$contentTime}::{$contentHash}"; } diff --git a/app/Theming/CustomHtmlHeadContentProvider.php b/app/Theming/CustomHtmlHeadContentProvider.php index dab30606c..9f794a077 100644 --- a/app/Theming/CustomHtmlHeadContentProvider.php +++ b/app/Theming/CustomHtmlHeadContentProvider.php @@ -41,7 +41,7 @@ class CustomHtmlHeadContentProvider $hash = md5($content); return $this->cache->remember('custom-head-export:' . $hash, 86400, function () use ($content) { - $config = new HtmlContentFilterConfig(filterOutNonContentElements: false); + $config = new HtmlContentFilterConfig(filterOutNonContentElements: false, useAllowListFilter: false); return (new HtmlContentFilter($config))->filterString($content); }); } diff --git a/app/Util/ConfiguredHtmlPurifier.php b/app/Util/ConfiguredHtmlPurifier.php index d63d2ad5f..014b2a3bf 100644 --- a/app/Util/ConfiguredHtmlPurifier.php +++ b/app/Util/ConfiguredHtmlPurifier.php @@ -62,7 +62,7 @@ class ConfiguredHtmlPurifier $config->set('Attr.EnableID', true); $config->set('Attr.ID.HTML5', true); $config->set('Output.FixInnerHTML', false); - $config->set('URI.SafeIframeRegexp', '%^(http://|https://)%'); + $config->set('URI.SafeIframeRegexp', '%^(http://|https://|//)%'); $config->set('URI.AllowedSchemes', [ 'http' => true, 'https' => true, diff --git a/app/Util/CspService.php b/app/Util/CspService.php index 4262b5c98..466acb491 100644 --- a/app/Util/CspService.php +++ b/app/Util/CspService.php @@ -65,7 +65,7 @@ class CspService */ protected function getScriptSrc(): string { - if (config('app.allow_content_scripts')) { + if ($this->scriptFilteringDisabled()) { return ''; } @@ -108,7 +108,7 @@ class CspService */ protected function getObjectSrc(): string { - if (config('app.allow_content_scripts')) { + if ($this->scriptFilteringDisabled()) { return ''; } @@ -124,6 +124,11 @@ class CspService return "base-uri 'self'"; } + protected function scriptFilteringDisabled(): bool + { + return !HtmlContentFilterConfig::fromConfigString(config('app.content_filtering'))->filterOutJavaScript; + } + protected function getAllowedIframeHosts(): array { $hosts = config('app.iframe_hosts') ?? ''; diff --git a/tests/Entity/PageContentFilteringTest.php b/tests/Entity/PageContentFilteringTest.php new file mode 100644 index 000000000..e1295034d --- /dev/null +++ b/tests/Entity/PageContentFilteringTest.php @@ -0,0 +1,353 @@ +asEditor(); + $page = $this->entities->page(); + $script = 'abc123abc123'; + $page->html = "escape {$script}"; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertDontSee($script, false); + $pageView->assertSee('abc123abc123'); + } + + public function test_more_complex_content_script_escaping_scenarios() + { + $checks = [ + "

Some script

", + "

Some script

", + "

Some script

", + "

Some script

", + "

Some script

", + "

Some script

", + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); + } + } + + public function test_js_and_base64_src_urls_are_removed() + { + $checks = [ + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $html = $this->withHtml($pageView); + $html->assertElementNotContains('.page-content', ''); + $html->assertElementNotContains('.page-content', 'src='); + $html->assertElementNotContains('.page-content', 'javascript:'); + $html->assertElementNotContains('.page-content', 'data:'); + $html->assertElementNotContains('.page-content', 'base64'); + } + } + + public function test_javascript_uri_links_are_removed() + { + $checks = [ + 'withHtml($pageView)->assertElementNotContains('.page-content', 'href=javascript:'); + } + } + + public function test_form_actions_with_javascript_are_removed() + { + $checks = [ + '', + 'Click me', + 'Click me', + '', + '', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertDontSee('id="xss"', false); + $pageView->assertDontSee('action=javascript:', false); + $pageView->assertDontSee('action=JaVaScRiPt:', false); + $pageView->assertDontSee('formaction=javascript:', false); + $pageView->assertDontSee('formaction=JaVaScRiPt:', false); + } + } + + public function test_form_elements_are_removed() + { + $checks = [ + '

thisisacattofind

thisdogshouldnotbefound
', + '

thisisacattofind

', + '

thisisacattofind

', + '

thisisacattofind

', + '

thisisacattofind

thisdogshouldnotbefound
', + '

thisisacattofind

', + '

thisisacattofind

', + <<<'TESTCASE' + + + + +

thisisacattofind

+
+

thisdogshouldnotbefound

+
+ + + + +
+
+TESTCASE + + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertSee('thisisacattofind'); + $pageView->assertDontSee('thisdogshouldnotbefound'); + } + } + + public function test_form_attributes_are_removed() + { + $withinSvgSample = <<<'TESTCASE' + + + + +

thisisacattofind

+

thisisacattofind

+ + +
+
+TESTCASE; + + $checks = [ + 'formaction' => '

thisisacattofind

', + 'form' => '

thisisacattofind

', + 'formmethod' => '

thisisacattofind

', + 'formtarget' => '

thisisacattofind

', + 'FORMTARGET' => '

thisisacattofind

', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $attribute => $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $pageView->assertSee('thisisacattofind'); + $this->withHtml($pageView)->assertElementNotExists(".page-content [{$attribute}]"); + } + + $page->html = $withinSvgSample; + $page->save(); + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $html = $this->withHtml($pageView); + foreach ($checks as $attribute => $check) { + $pageView->assertSee('thisisacattofind'); + $html->assertElementNotExists(".page-content [{$attribute}]"); + } + } + + public function test_metadata_redirects_are_removed() + { + $checks = [ + '', + '', + '', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); + $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); + $this->withHtml($pageView)->assertElementNotContains('.page-content', 'content='); + $this->withHtml($pageView)->assertElementNotContains('.page-content', 'external_url'); + } + } + + public function test_page_inline_on_attributes_removed_by_default() + { + $this->asEditor(); + $page = $this->entities->page(); + $script = '

Hello

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

Hello

', false); + } + + public function test_more_complex_inline_on_attributes_escaping_scenarios() + { + $checks = [ + '

Hello

', + '

Hello

', + '
Lorem ipsum dolor sit amet.

Hello

', + '
Lorem ipsum dolor sit amet.

Hello

', + '
Lorem ipsum dolor sit amet.

Hello

', + '
Lorem ipsum dolor sit amet.

Hello

', + '
xss link\', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $this->withHtml($pageView)->assertElementNotContains('.page-content', 'onclick'); + } + } + + public function test_page_content_scripts_show_with_filters_disabled() + { + $this->asEditor(); + $page = $this->entities->page(); + config()->set('app.content_filtering', ''); + + $script = 'abc123abc123'; + $page->html = "no escape {$script}"; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertSee($script, false); + $pageView->assertDontSee('abc123abc123'); + } + + public function test_svg_script_usage_is_removed() + { + $checks = [ + '', + '', + '', + '', + '', + 'XSS', + 'XSS', + '', + ]; + + $this->asEditor(); + $page = $this->entities->page(); + + foreach ($checks as $check) { + $page->html = $check; + $page->save(); + + $pageView = $this->get($page->getUrl()); + $pageView->assertStatus(200); + $html = $this->withHtml($pageView); + $html->assertElementNotContains('.page-content', 'alert'); + $html->assertElementNotContains('.page-content', 'xlink:href'); + $html->assertElementNotContains('.page-content', 'application/xml'); + $html->assertElementNotContains('.page-content', 'javascript'); + } + } + + public function test_page_inline_on_attributes_show_with_filters_disabled() + { + $this->asEditor(); + $page = $this->entities->page(); + config()->set('app.content_filtering', ''); + + $script = '

Hello

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

Hello

', false); + } +} diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 770261130..deae153e1 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -101,351 +101,6 @@ class PageContentTest extends TestCase $pageResp->assertSee('Hello Barry'); } - public function test_page_content_scripts_removed_by_default() - { - $this->asEditor(); - $page = $this->entities->page(); - $script = 'abc123abc123'; - $page->html = "escape {$script}"; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $pageView->assertDontSee($script, false); - $pageView->assertSee('abc123abc123'); - } - - public function test_more_complex_content_script_escaping_scenarios() - { - $checks = [ - "

Some script

", - "

Some script

", - "

Some script

", - "

Some script

", - "

Some script

", - "

Some script

", - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); - } - } - - public function test_js_and_base64_src_urls_are_removed() - { - $checks = [ - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - '', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $html = $this->withHtml($pageView); - $html->assertElementNotContains('.page-content', ''); - $html->assertElementNotContains('.page-content', 'src='); - $html->assertElementNotContains('.page-content', 'javascript:'); - $html->assertElementNotContains('.page-content', 'data:'); - $html->assertElementNotContains('.page-content', 'base64'); - } - } - - public function test_javascript_uri_links_are_removed() - { - $checks = [ - '
withHtml($pageView)->assertElementNotContains('.page-content', 'href=javascript:'); - } - } - - public function test_form_actions_with_javascript_are_removed() - { - $checks = [ - '', - 'Click me', - 'Click me', - '', - '', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $pageView->assertDontSee('id="xss"', false); - $pageView->assertDontSee('action=javascript:', false); - $pageView->assertDontSee('action=JaVaScRiPt:', false); - $pageView->assertDontSee('formaction=javascript:', false); - $pageView->assertDontSee('formaction=JaVaScRiPt:', false); - } - } - - public function test_form_elements_are_removed() - { - $checks = [ - '

thisisacattofind

thisdogshouldnotbefound
', - '

thisisacattofind

', - '

thisisacattofind

', - '

thisisacattofind

', - '

thisisacattofind

thisdogshouldnotbefound
', - '

thisisacattofind

', - '

thisisacattofind

', - <<<'TESTCASE' - - - - -

thisisacattofind

-
-

thisdogshouldnotbefound

-
- - - - -
-
-TESTCASE - - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $pageView->assertSee('thisisacattofind'); - $pageView->assertDontSee('thisdogshouldnotbefound'); - } - } - - public function test_form_attributes_are_removed() - { - $withinSvgSample = <<<'TESTCASE' - - - - -

thisisacattofind

-

thisisacattofind

- - -
-
-TESTCASE; - - $checks = [ - 'formaction' => '

thisisacattofind

', - 'form' => '

thisisacattofind

', - 'formmethod' => '

thisisacattofind

', - 'formtarget' => '

thisisacattofind

', - 'FORMTARGET' => '

thisisacattofind

', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $attribute => $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $pageView->assertSee('thisisacattofind'); - $this->withHtml($pageView)->assertElementNotExists(".page-content [{$attribute}]"); - } - - $page->html = $withinSvgSample; - $page->save(); - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $html = $this->withHtml($pageView); - foreach ($checks as $attribute => $check) { - $pageView->assertSee('thisisacattofind'); - $html->assertElementNotExists(".page-content [{$attribute}]"); - } - } - - public function test_metadata_redirects_are_removed() - { - $checks = [ - '', - '', - '', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); - $this->withHtml($pageView)->assertElementNotContains('.page-content', ''); - $this->withHtml($pageView)->assertElementNotContains('.page-content', 'content='); - $this->withHtml($pageView)->assertElementNotContains('.page-content', 'external_url'); - } - } - - public function test_page_inline_on_attributes_removed_by_default() - { - $this->asEditor(); - $page = $this->entities->page(); - $script = '

Hello

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

Hello

', false); - } - - public function test_more_complex_inline_on_attributes_escaping_scenarios() - { - $checks = [ - '

Hello

', - '

Hello

', - '
Lorem ipsum dolor sit amet.

Hello

', - '
Lorem ipsum dolor sit amet.

Hello

', - '
Lorem ipsum dolor sit amet.

Hello

', - '
Lorem ipsum dolor sit amet.

Hello

', - '
xss link\', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $this->withHtml($pageView)->assertElementNotContains('.page-content', 'onclick'); - } - } - - public function test_page_content_scripts_show_when_configured() - { - $this->asEditor(); - $page = $this->entities->page(); - config()->set('app.allow_content_scripts', 'true'); - - $script = 'abc123abc123'; - $page->html = "no escape {$script}"; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertSee($script, false); - $pageView->assertDontSee('abc123abc123'); - } - - public function test_svg_script_usage_is_removed() - { - $checks = [ - '', - '', - '', - '', - '', - 'XSS', - 'XSS', - '', - ]; - - $this->asEditor(); - $page = $this->entities->page(); - - foreach ($checks as $check) { - $page->html = $check; - $page->save(); - - $pageView = $this->get($page->getUrl()); - $pageView->assertStatus(200); - $html = $this->withHtml($pageView); - $html->assertElementNotContains('.page-content', 'alert'); - $html->assertElementNotContains('.page-content', 'xlink:href'); - $html->assertElementNotContains('.page-content', 'application/xml'); - $html->assertElementNotContains('.page-content', 'javascript'); - } - } - - public function test_page_inline_on_attributes_show_if_configured() - { - $this->asEditor(); - $page = $this->entities->page(); - config()->set('app.allow_content_scripts', 'true'); - - $script = '

Hello

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

Hello

', false); - } - public function test_duplicate_ids_does_not_break_page_render() { $this->asEditor(); @@ -649,6 +304,7 @@ TESTCASE; public function test_page_markdown_single_html_comment_saving() { + config()->set('app.content_filtering', 'jfh'); $this->asEditor(); $page = $this->entities->page(); @@ -656,7 +312,7 @@ TESTCASE; $this->put($page->getUrl(), [ 'name' => $page->name, 'markdown' => $content, 'html' => '', 'summary' => '', - ]); + ])->assertRedirect(); $page->refresh(); $this->assertStringMatchesFormat($content, $page->html); diff --git a/tests/SecurityHeaderTest.php b/tests/SecurityHeaderTest.php index fe98e3208..3f4b7d193 100644 --- a/tests/SecurityHeaderTest.php +++ b/tests/SecurityHeaderTest.php @@ -93,14 +93,14 @@ class SecurityHeaderTest extends TestCase $this->assertNotEquals($firstHeader, $secondHeader); } - public function test_allow_content_scripts_settings_controls_csp_script_headers() + public function test_content_filtering_config_controls_csp_script_headers() { - config()->set('app.allow_content_scripts', true); + config()->set('app.content_filtering', ''); $resp = $this->get('/'); $scriptHeader = $this->getCspHeader($resp, 'script-src'); $this->assertEmpty($scriptHeader); - config()->set('app.allow_content_scripts', false); + config()->set('app.content_filtering', 'j'); $resp = $this->get('/'); $scriptHeader = $this->getCspHeader($resp, 'script-src'); $this->assertNotEmpty($scriptHeader);