Skip to content

Commit f07dd35

Browse files
jasonvargaclaude
jasonvarga
and
claude
authored
[6.x] Centralize SVG sanitization and sanitize CSS in style tags (#14442)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5abd093 commit f07dd35

File tree

7 files changed

+225
-18
lines changed

7 files changed

+225
-18
lines changed

‎src/Assets/Asset.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use Illuminate\Support\Carbon;
99
use Illuminate\Support\Facades\Cache;
1010
use League\Flysystem\PathTraversalDetected;
11-
use Rhukster\DomSanitizer\DOMSanitizer;
1211
use Statamic\Assets\AssetUploader as Uploader;
1312
use Statamic\Contracts\Assets\Asset as AssetContract;
1413
use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract;
@@ -46,6 +45,7 @@
4645
use Statamic\Statamic;
4746
use Statamic\Support\Arr;
4847
use Statamic\Support\Str;
48+
use Statamic\Support\Svg;
4949
use Statamic\Support\Traits\FluentlyGetsAndSets;
5050
use Statamic\Support\Traits\Hookable;
5151
use Symfony\Component\HttpFoundation\File\UploadedFile;
@@ -970,9 +970,7 @@ public function reupload(ReplacementFile $file)
970970

971971
$this->disk()->put(
972972
$this->path(),
973-
(new DOMSanitizer(DOMSanitizer::SVG))->sanitize($contents, [
974-
'remove-xml-tags' => ! Str::startsWith($contents, '<?xml'),
975-
])
973+
Svg::sanitize($contents)
976974
);
977975
}
978976

‎src/Assets/Uploader.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
namespace Statamic\Assets;
44

55
use Facades\Statamic\Imaging\ImageValidator;
6-
use Rhukster\DomSanitizer\DOMSanitizer;
76
use Statamic\Facades\Glide;
87
use Statamic\Support\Str;
8+
use Statamic\Support\Svg;
99
use Symfony\Component\HttpFoundation\File\UploadedFile;
1010

1111
abstract class Uploader
@@ -59,10 +59,7 @@ private function write($sourcePath, $destinationPath)
5959
$stream = fopen($sourcePath, 'r');
6060

6161
if (config('statamic.assets.svg_sanitization_on_upload', true) && Str::endsWith($destinationPath, '.svg')) {
62-
$sanitizer = new DOMSanitizer(DOMSanitizer::SVG);
63-
$stream = $sanitizer->sanitize($svg = stream_get_contents($stream), [
64-
'remove-xml-tags' => ! Str::startsWith($svg, '<?xml'),
65-
]);
62+
$stream = Svg::sanitize(stream_get_contents($stream));
6663
}
6764

6865
$this->disk()->put($this->uploadPathPrefix().$destinationPath, $stream);

‎src/CP/Navigation/NavItem.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace Statamic\CP\Navigation;
44

55
use Illuminate\Support\Collection;
6-
use Rhukster\DomSanitizer\DOMSanitizer;
76
use Statamic\CommandPalette\Category;
87
use Statamic\CommandPalette\Link;
98
use Statamic\Facades\CP\Nav;
@@ -217,11 +216,7 @@ public function svg()
217216
private function sanitizeSvg(string $svg): string
218217
{
219218
try {
220-
$sanitizer = new DOMSanitizer(DOMSanitizer::SVG);
221-
222-
return $sanitizer->sanitize($svg, [
223-
'remove-xml-tags' => ! Str::startsWith($svg, '<?xml'),
224-
]);
219+
return Svg::sanitize($svg);
225220
} catch (\Throwable $e) {
226221
return '';
227222
}

‎src/Support/Svg.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Statamic\Support;
44

5+
use Rhukster\DomSanitizer\DOMSanitizer;
56
use Stringy\StaticStringy;
67

78
class Svg
@@ -14,4 +15,47 @@ public static function withClasses(string $svg, ?string $classes = null): string
1415

1516
return str_replace('<svg', sprintf('<svg%s', $attrs), $svg);
1617
}
18+
19+
public static function sanitize(string $svg, ?DOMSanitizer $sanitizer = null): string
20+
{
21+
$sanitizer = $sanitizer ?? new DOMSanitizer(DOMSanitizer::SVG);
22+
23+
$svg = $sanitizer->sanitize($svg, [
24+
'remove-xml-tags' => ! Str::startsWith($svg, '<?xml'),
25+
]);
26+
27+
return static::sanitizeStyleTags($svg);
28+
}
29+
30+
public static function sanitizeCss(string $css): string
31+
{
32+
// Decode all CSS escape sequences in a single pass to prevent bypass.
33+
// Hex escapes: \69mport -> import. Non-hex escapes: \i -> i, \@ -> @.
34+
$css = preg_replace_callback(
35+
'/\\\\(?:([0-9a-fA-F]{1,6})\s?|(.))/s',
36+
fn ($m) => ($m[1] !== '') ? mb_chr(hexdec($m[1]), 'UTF-8') : $m[2],
37+
$css
38+
);
39+
40+
// Normalize Unicode whitespace and invisible characters to ASCII spaces
41+
// so they can't be used to sneak past the regex patterns below
42+
$css = preg_replace('/[\p{Z}\x{200B}\x{FEFF}]+/u', ' ', $css);
43+
44+
// Remove @import rules entirely
45+
$css = preg_replace('/@import\s+[^;]+;?/i', '', $css);
46+
47+
// Neutralize url() references to external resources (http, https, protocol-relative)
48+
$css = preg_replace('/url\s*\(\s*["\']?\s*(?:https?:|\/\/)[^)]*\)/i', 'url()', $css);
49+
50+
return $css;
51+
}
52+
53+
private static function sanitizeStyleTags(string $svg): string
54+
{
55+
return preg_replace_callback(
56+
'/<style([^>]*)>(.*?)<\/style>/si',
57+
fn ($matches) => '<style'.$matches[1].'>'.static::sanitizeCss($matches[2]).'</style>',
58+
$svg
59+
);
60+
}
1761
}

‎src/Tags/Svg.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Statamic\Facades\File;
77
use Statamic\Facades\Path;
88
use Statamic\Support\Str;
9+
use Statamic\Support\Svg as SvgSupport;
910
use Stringy\StaticStringy;
1011

1112
class Svg extends Tags
@@ -105,9 +106,7 @@ private function sanitize($svg)
105106
$this->setAllowedAttrs($sanitizer);
106107
$this->setAllowedTags($sanitizer);
107108

108-
return $sanitizer->sanitize($svg, [
109-
'remove-xml-tags' => ! Str::startsWith($svg, '<?xml'),
110-
]);
109+
return SvgSupport::sanitize($svg, $sanitizer);
111110
}
112111

113112
private function setAllowedAttrs(DOMSanitizer $sanitizer)

‎tests/Support/SvgTest.php‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
<?php
2+
3+
namespace Tests\Support;
4+
5+
use PHPUnit\Framework\Attributes\DataProvider;
6+
use PHPUnit\Framework\Attributes\Test;
7+
use Statamic\Support\Svg;
8+
use Tests\TestCase;
9+
10+
class SvgTest extends TestCase
11+
{
12+
#[Test]
13+
#[DataProvider('sanitizeCssProvider')]
14+
public function it_sanitizes_css(string $input, string $expected)
15+
{
16+
$this->assertSame($expected, trim(Svg::sanitizeCss($input)));
17+
}
18+
19+
public static function sanitizeCssProvider()
20+
{
21+
return [
22+
'strips @import with url()' => [
23+
'@import url("https://evil.com/x.css");',
24+
'',
25+
],
26+
'strips @import with bare string' => [
27+
'@import "https://evil.com/x.css";',
28+
'',
29+
],
30+
'strips @import with protocol-relative url' => [
31+
'@import url(//evil.com/x.css);',
32+
'',
33+
],
34+
'strips @import without semicolon' => [
35+
"@import url('https://evil.com/x.css')",
36+
'',
37+
],
38+
'strips @import using hex escapes' => [
39+
'@\\69mport url("https://evil.com/x.css");',
40+
'',
41+
],
42+
'strips @import using non-hex backslash escapes' => [
43+
'@\import url("https://evil.com/x.css");',
44+
'',
45+
],
46+
'strips @import using mixed hex and non-hex escapes' => [
47+
'@\\69\mport url("https://evil.com/x.css");',
48+
'',
49+
],
50+
'neutralizes external url' => [
51+
'.cls { background: url(https://evil.com/beacon.gif); }',
52+
'.cls { background: url(); }',
53+
],
54+
'neutralizes protocol-relative url' => [
55+
'.cls { background: url(//evil.com/x); }',
56+
'.cls { background: url(); }',
57+
],
58+
'neutralizes quoted external url' => [
59+
'.cls { background: url("http://evil.com/x"); }',
60+
'.cls { background: url(); }',
61+
],
62+
'neutralizes external url using hex escapes' => [
63+
'.cls { background: url(\\68\\74\\74\\70\\73://evil.com/beacon.gif); }',
64+
'.cls { background: url(); }',
65+
],
66+
'neutralizes external url using non-hex backslash escapes' => [
67+
'.cls { background: url(\https://evil.com/x); }',
68+
'.cls { background: url(); }',
69+
],
70+
'neutralizes external url using non-breaking space escape' => [
71+
'.cls { background: url(\\a0 https://evil.com/x); }',
72+
'.cls { background: url(); }',
73+
],
74+
'neutralizes external url using zero-width space escape' => [
75+
'.cls { background: url(\\200B https://evil.com/x); }',
76+
'.cls { background: url(); }',
77+
],
78+
'neutralizes external url using BOM escape' => [
79+
'.cls { background: url(\\FEFF https://evil.com/x); }',
80+
'.cls { background: url(); }',
81+
],
82+
'neutralizes external url in @font-face src' => [
83+
'@font-face { font-family: "x"; src: url("https://evil.com/font.woff"); }',
84+
'@font-face { font-family: "x"; src: url(); }',
85+
],
86+
'preserves normal css' => [
87+
'.cls-1 { fill: #333; stroke: red; }',
88+
'.cls-1 { fill: #333; stroke: red; }',
89+
],
90+
'preserves internal url references' => [
91+
'.cls { fill: url(#myGradient); }',
92+
'.cls { fill: url(#myGradient); }',
93+
],
94+
'preserves data uris' => [
95+
'.cls { background: url(data:image/png;base64,abc123); }',
96+
'.cls { background: url(data:image/png;base64,abc123); }',
97+
],
98+
'handles mixed legitimate and malicious css' => [
99+
".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); }",
100+
".cls-1 { fill: #333; }\n\n.cls-2 { stroke: url(#grad); background: url(); }",
101+
],
102+
];
103+
}
104+
105+
#[Test]
106+
public function it_sanitizes_style_tags_in_full_svg()
107+
{
108+
$svg = '<svg xmlns="http://www.w3.org/2000/svg"><style>@import url("https://evil.com/track.css"); .cls-1 { fill: #333; }</style><rect class="cls-1"/></svg>';
109+
110+
$result = Svg::sanitize($svg);
111+
112+
$this->assertStringNotContainsString('@import', $result);
113+
$this->assertStringNotContainsString('evil.com', $result);
114+
$this->assertStringContainsString('.cls-1', $result);
115+
$this->assertStringContainsString('fill:', $result);
116+
}
117+
118+
#[Test]
119+
public function it_passes_through_svg_without_style_tags()
120+
{
121+
$svg = '<svg xmlns="http://www.w3.org/2000/svg"><rect width="1" height="1" fill="white"/></svg>';
122+
123+
$result = Svg::sanitize($svg);
124+
125+
$this->assertStringContainsString('<rect', $result);
126+
$this->assertStringContainsString('<svg', $result);
127+
}
128+
129+
#[Test]
130+
public function it_preserves_xml_declaration()
131+
{
132+
$svg = '<?xml version="1.0" encoding="UTF-8"?><svg xmlns="http://www.w3.org/2000/svg"><rect/></svg>';
133+
134+
$result = Svg::sanitize($svg);
135+
136+
$this->assertStringStartsWith('<?xml', $result);
137+
}
138+
139+
#[Test]
140+
public function it_does_not_add_xml_declaration()
141+
{
142+
$svg = '<svg xmlns="http://www.w3.org/2000/svg"><rect/></svg>';
143+
144+
$result = Svg::sanitize($svg);
145+
146+
$this->assertStringStartsWith('<svg', $result);
147+
}
148+
149+
#[Test]
150+
public function it_sanitizes_css_inside_cdata_sections()
151+
{
152+
$svg = '<svg xmlns="http://www.w3.org/2000/svg"><style><![CDATA[@import url("https://evil.com/track.css"); .cls-1 { fill: url(https://evil.com/bg.gif); }]]></style><rect class="cls-1"/></svg>';
153+
154+
$result = Svg::sanitize($svg);
155+
156+
$this->assertStringNotContainsString('@import', $result);
157+
$this->assertStringNotContainsString('evil.com', $result);
158+
$this->assertStringContainsString('.cls-1', $result);
159+
$this->assertStringContainsString('fill:', $result);
160+
}
161+
}

‎tests/Tags/SvgTagTest.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,19 @@ public function sanitization_can_be_disabled()
114114
$this->assertEquals('<svg><path/></svg>', $this->tag('{{ svg src="xss" }}'));
115115
}
116116

117+
#[Test]
118+
public function it_sanitizes_css_in_style_tags()
119+
{
120+
File::put(resource_path('css-inject.svg'), '<svg xmlns="http://www.w3.org/2000/svg"><style>@import url("https://evil.com/track.css"); .cls-1 { fill: #333; }</style><rect class="cls-1"/></svg>');
121+
122+
$result = $this->tag('{{ svg src="css-inject" }}');
123+
124+
$this->assertStringNotContainsString('@import', $result);
125+
$this->assertStringNotContainsString('evil.com', $result);
126+
$this->assertStringContainsString('.cls-1', $result);
127+
$this->assertStringContainsString('fill:', $result);
128+
}
129+
117130
#[Test]
118131
public function fails_gracefully_when_src_is_empty()
119132
{

0 commit comments

Comments
 (0)