Skip to content

Commit ecc1caa

Browse files
[5.x] Sanitize SVGs on asset reupload (#14270)
Co-authored-by: Duncan McClean <duncan@duncanmcclean.com>
1 parent 5b0c01c commit ecc1caa

2 files changed

Lines changed: 82 additions & 0 deletions

File tree

src/Assets/Asset.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Illuminate\Support\Carbon;
99
use Illuminate\Support\Facades\Cache;
1010
use League\Flysystem\PathTraversalDetected;
11+
use Rhukster\DomSanitizer\DOMSanitizer;
1112
use Statamic\Assets\AssetUploader as Uploader;
1213
use Statamic\Contracts\Assets\Asset as AssetContract;
1314
use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract;
@@ -945,6 +946,17 @@ public function reupload(ReplacementFile $file)
945946

946947
$file->writeTo($this->disk()->filesystem(), $this->path());
947948

949+
if ($this->isSvg() && config('statamic.assets.svg_sanitization_on_upload', true)) {
950+
$contents = $this->disk()->get($this->path());
951+
952+
$this->disk()->put(
953+
$this->path(),
954+
(new DOMSanitizer(DOMSanitizer::SVG))->sanitize($contents, [
955+
'remove-xml-tags' => ! Str::startsWith($contents, '<?xml'),
956+
])
957+
);
958+
}
959+
948960
$this->clearCaches();
949961
$this->writeMeta($this->generateMeta());
950962

tests/Assets/AssetTest.php

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2180,6 +2180,76 @@ public function cannot_reupload_a_file_with_a_different_extension()
21802180
Event::assertNotDispatched(AssetSaved::class);
21812181
}
21822182

2183+
#[Test]
2184+
public function it_sanitizes_svgs_on_reupload()
2185+
{
2186+
Event::fake();
2187+
2188+
// Create and upload an initial clean SVG
2189+
$asset = (new Asset)->container($this->container)->path('path/to/asset.svg')->syncOriginal();
2190+
Facades\AssetContainer::shouldReceive('findByHandle')->with('test_container')->andReturn($this->container);
2191+
$asset->upload(UploadedFile::fake()->createWithContent('asset.svg', '<svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"></svg>'));
2192+
Storage::disk('test')->assertExists('path/to/asset.svg');
2193+
2194+
// Place a malicious SVG in the local disk for reupload
2195+
$uploadDisk = Storage::fake('local');
2196+
$maliciousSvg = '<?xml version="1.0" encoding="UTF-8" standalone="no"?><svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"><script type="text/javascript">alert(`Bad stuff could go in here.`);</script></svg>';
2197+
$uploadDisk->put('path/to/malicious.svg', $maliciousSvg);
2198+
$uploadDisk->assertExists('path/to/malicious.svg');
2199+
2200+
$file = new ReplacementFile('path/to/malicious.svg');
2201+
2202+
$return = $asset->reupload($file);
2203+
2204+
$this->assertEquals($asset, $return);
2205+
Storage::disk('test')->assertExists('path/to/asset.svg');
2206+
2207+
// Ensure the inline scripts were stripped out
2208+
$this->assertStringNotContainsString('<script', $asset->contents());
2209+
$this->assertStringNotContainsString('Bad stuff could go in here.', $asset->contents());
2210+
$this->assertStringNotContainsString('</script>', $asset->contents());
2211+
2212+
Event::assertDispatched(AssetReuploaded::class, function ($event) use ($asset) {
2213+
return $event->asset->id() === $asset->id();
2214+
});
2215+
}
2216+
2217+
#[Test]
2218+
public function it_does_not_sanitize_svgs_on_reupload_when_behaviour_is_disabled()
2219+
{
2220+
Event::fake();
2221+
2222+
config()->set('statamic.assets.svg_sanitization_on_upload', false);
2223+
2224+
// Create and upload an initial clean SVG
2225+
$asset = (new Asset)->container($this->container)->path('path/to/asset.svg')->syncOriginal();
2226+
Facades\AssetContainer::shouldReceive('findByHandle')->with('test_container')->andReturn($this->container);
2227+
$asset->upload(UploadedFile::fake()->createWithContent('asset.svg', '<svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"></svg>'));
2228+
Storage::disk('test')->assertExists('path/to/asset.svg');
2229+
2230+
// Place a malicious SVG in the local disk for reupload
2231+
$uploadDisk = Storage::fake('local');
2232+
$maliciousSvg = '<?xml version="1.0" encoding="UTF-8" standalone="no"?><svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"><script type="text/javascript">alert(`Bad stuff could go in here.`);</script></svg>';
2233+
$uploadDisk->put('path/to/malicious.svg', $maliciousSvg);
2234+
$uploadDisk->assertExists('path/to/malicious.svg');
2235+
2236+
$file = new ReplacementFile('path/to/malicious.svg');
2237+
2238+
$return = $asset->reupload($file);
2239+
2240+
$this->assertEquals($asset, $return);
2241+
Storage::disk('test')->assertExists('path/to/asset.svg');
2242+
2243+
// Ensure the inline scripts were NOT stripped out when disabled
2244+
$this->assertStringContainsString('<script', $asset->contents());
2245+
$this->assertStringContainsString('Bad stuff could go in here.', $asset->contents());
2246+
$this->assertStringContainsString('</script>', $asset->contents());
2247+
2248+
Event::assertDispatched(AssetReuploaded::class, function ($event) use ($asset) {
2249+
return $event->asset->id() === $asset->id();
2250+
});
2251+
}
2252+
21832253
#[Test]
21842254
public function it_doesnt_lowercase_uploaded_filenames_when_configured()
21852255
{

0 commit comments

Comments
 (0)