diff --git a/src/Assets/Asset.php b/src/Assets/Asset.php index f820b780c9b..86ba83351e5 100644 --- a/src/Assets/Asset.php +++ b/src/Assets/Asset.php @@ -8,7 +8,6 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use League\Flysystem\PathTraversalDetected; -use Rhukster\DomSanitizer\DOMSanitizer; use Statamic\Assets\AssetUploader as Uploader; use Statamic\Contracts\Assets\Asset as AssetContract; use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; @@ -46,6 +45,7 @@ use Statamic\Statamic; use Statamic\Support\Arr; use Statamic\Support\Str; +use Statamic\Support\Svg; use Statamic\Support\Traits\FluentlyGetsAndSets; use Statamic\Support\Traits\Hookable; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -970,9 +970,7 @@ public function reupload(ReplacementFile $file) $this->disk()->put( $this->path(), - (new DOMSanitizer(DOMSanitizer::SVG))->sanitize($contents, [ - 'remove-xml-tags' => ! Str::startsWith($contents, 'sanitize($svg = stream_get_contents($stream), [ - 'remove-xml-tags' => ! Str::startsWith($svg, 'disk()->put($this->uploadPathPrefix().$destinationPath, $stream); diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php index eca3577846f..55d322b0d6d 100644 --- a/src/CP/Navigation/NavItem.php +++ b/src/CP/Navigation/NavItem.php @@ -3,7 +3,6 @@ namespace Statamic\CP\Navigation; use Illuminate\Support\Collection; -use Rhukster\DomSanitizer\DOMSanitizer; use Statamic\CommandPalette\Category; use Statamic\CommandPalette\Link; use Statamic\Facades\CP\Nav; @@ -217,11 +216,7 @@ public function svg() private function sanitizeSvg(string $svg): string { try { - $sanitizer = new DOMSanitizer(DOMSanitizer::SVG); - - return $sanitizer->sanitize($svg, [ - 'remove-xml-tags' => ! Str::startsWith($svg, 'sanitize($svg, [ + 'remove-xml-tags' => ! Str::startsWith($svg, ' import. Non-hex escapes: \i -> i, \@ -> @. + $css = preg_replace_callback( + '/\\\\(?:([0-9a-fA-F]{1,6})\s?|(.))/s', + fn ($m) => ($m[1] !== '') ? mb_chr(hexdec($m[1]), 'UTF-8') : $m[2], + $css + ); + + // Normalize Unicode whitespace and invisible characters to ASCII spaces + // so they can't be used to sneak past the regex patterns below + $css = preg_replace('/[\p{Z}\x{200B}\x{FEFF}]+/u', ' ', $css); + + // Remove @import rules entirely + $css = preg_replace('/@import\s+[^;]+;?/i', '', $css); + + // Neutralize url() references to external resources (http, https, protocol-relative) + $css = preg_replace('/url\s*\(\s*["\']?\s*(?:https?:|\/\/)[^)]*\)/i', 'url()', $css); + + return $css; + } + + private static function sanitizeStyleTags(string $svg): string + { + return preg_replace_callback( + '/]*)>(.*?)<\/style>/si', + fn ($matches) => ''.static::sanitizeCss($matches[2]).'', + $svg + ); + } } diff --git a/src/Tags/Svg.php b/src/Tags/Svg.php index 5bf511455ec..78300c4e70d 100644 --- a/src/Tags/Svg.php +++ b/src/Tags/Svg.php @@ -6,6 +6,7 @@ use Statamic\Facades\File; use Statamic\Facades\Path; use Statamic\Support\Str; +use Statamic\Support\Svg as SvgSupport; use Stringy\StaticStringy; class Svg extends Tags @@ -105,9 +106,7 @@ private function sanitize($svg) $this->setAllowedAttrs($sanitizer); $this->setAllowedTags($sanitizer); - return $sanitizer->sanitize($svg, [ - 'remove-xml-tags' => ! Str::startsWith($svg, 'assertSame($expected, trim(Svg::sanitizeCss($input))); + } + + public static function sanitizeCssProvider() + { + return [ + 'strips @import with url()' => [ + '@import url("https://evil.com/x.css");', + '', + ], + 'strips @import with bare string' => [ + '@import "https://evil.com/x.css";', + '', + ], + 'strips @import with protocol-relative url' => [ + '@import url(//evil.com/x.css);', + '', + ], + 'strips @import without semicolon' => [ + "@import url('https://evil.com/x.css')", + '', + ], + 'strips @import using hex escapes' => [ + '@\\69mport url("https://evil.com/x.css");', + '', + ], + 'strips @import using non-hex backslash escapes' => [ + '@\import url("https://evil.com/x.css");', + '', + ], + 'strips @import using mixed hex and non-hex escapes' => [ + '@\\69\mport url("https://evil.com/x.css");', + '', + ], + 'neutralizes external url' => [ + '.cls { background: url(https://evil.com/beacon.gif); }', + '.cls { background: url(); }', + ], + 'neutralizes protocol-relative url' => [ + '.cls { background: url(//evil.com/x); }', + '.cls { background: url(); }', + ], + 'neutralizes quoted external url' => [ + '.cls { background: url("http://evil.com/x"); }', + '.cls { background: url(); }', + ], + 'neutralizes external url using hex escapes' => [ + '.cls { background: url(\\68\\74\\74\\70\\73://evil.com/beacon.gif); }', + '.cls { background: url(); }', + ], + 'neutralizes external url using non-hex backslash escapes' => [ + '.cls { background: url(\https://evil.com/x); }', + '.cls { background: url(); }', + ], + 'neutralizes external url using non-breaking space escape' => [ + '.cls { background: url(\\a0 https://evil.com/x); }', + '.cls { background: url(); }', + ], + 'neutralizes external url using zero-width space escape' => [ + '.cls { background: url(\\200B https://evil.com/x); }', + '.cls { background: url(); }', + ], + 'neutralizes external url using BOM escape' => [ + '.cls { background: url(\\FEFF https://evil.com/x); }', + '.cls { background: url(); }', + ], + 'neutralizes external url in @font-face src' => [ + '@font-face { font-family: "x"; src: url("https://evil.com/font.woff"); }', + '@font-face { font-family: "x"; src: url(); }', + ], + 'preserves normal css' => [ + '.cls-1 { fill: #333; stroke: red; }', + '.cls-1 { fill: #333; stroke: red; }', + ], + 'preserves internal url references' => [ + '.cls { fill: url(#myGradient); }', + '.cls { fill: url(#myGradient); }', + ], + 'preserves data uris' => [ + '.cls { background: url(data:image/png;base64,abc123); }', + '.cls { background: url(data:image/png;base64,abc123); }', + ], + 'handles mixed legitimate and malicious css' => [ + ".cls-1 { fill: #333; }\n@import url(\"https://evil.com/track.css\");\n.cls-2 { stroke: url(#grad); background: url(https://evil.com/bg.gif); }", + ".cls-1 { fill: #333; }\n\n.cls-2 { stroke: url(#grad); background: url(); }", + ], + ]; + } + + #[Test] + public function it_sanitizes_style_tags_in_full_svg() + { + $svg = ''; + + $result = Svg::sanitize($svg); + + $this->assertStringNotContainsString('@import', $result); + $this->assertStringNotContainsString('evil.com', $result); + $this->assertStringContainsString('.cls-1', $result); + $this->assertStringContainsString('fill:', $result); + } + + #[Test] + public function it_passes_through_svg_without_style_tags() + { + $svg = ''; + + $result = Svg::sanitize($svg); + + $this->assertStringContainsString('assertStringContainsString('assertStringStartsWith(''; + + $result = Svg::sanitize($svg); + + $this->assertStringStartsWith('assertStringNotContainsString('@import', $result); + $this->assertStringNotContainsString('evil.com', $result); + $this->assertStringContainsString('.cls-1', $result); + $this->assertStringContainsString('fill:', $result); + } +} diff --git a/tests/Tags/SvgTagTest.php b/tests/Tags/SvgTagTest.php index 181c30d5038..0033285061e 100644 --- a/tests/Tags/SvgTagTest.php +++ b/tests/Tags/SvgTagTest.php @@ -114,6 +114,19 @@ public function sanitization_can_be_disabled() $this->assertEquals('', $this->tag('{{ svg src="xss" }}')); } + #[Test] + public function it_sanitizes_css_in_style_tags() + { + File::put(resource_path('css-inject.svg'), ''); + + $result = $this->tag('{{ svg src="css-inject" }}'); + + $this->assertStringNotContainsString('@import', $result); + $this->assertStringNotContainsString('evil.com', $result); + $this->assertStringContainsString('.cls-1', $result); + $this->assertStringContainsString('fill:', $result); + } + #[Test] public function fails_gracefully_when_src_is_empty() {