Skip to content

Commit 2df5847

Browse files
author
Chris Hunt
committed
Fix uploads and displaying of malicious SVG files
1 parent d1801d1 commit 2df5847

29 files changed

Lines changed: 25060 additions & 16008 deletions

‎.phpstorm.meta.php‎

Lines changed: 1705 additions & 1496 deletions
Large diffs are not rendered by default.

‎_ide_helper.php‎

Lines changed: 20702 additions & 14486 deletions
Large diffs are not rendered by default.

‎app/Helper.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

33
use Illuminate\Support\Str;
4+
use enshrined\svgSanitize\Sanitizer;
45

56
/**
67
* @param $bytes
@@ -129,7 +130,11 @@ function isImage(string $file, string $extension): bool
129130
fwrite($handle, $file);
130131
fclose($handle);
131132

132-
if ($extension == 'svg') {
133+
if ($extension === 'svg') {
134+
$sanitizer = new Sanitizer();
135+
$sanitizedSvg = $sanitizer->sanitize(file_get_contents($tempFileName));
136+
file_put_contents($tempFileName, $sanitizedSvg);
137+
133138
return 'image/svg+xml' === mime_content_type($tempFileName);
134139
}
135140

‎app/Http/Controllers/ItemController.php‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Illuminate\Validation\ValidationException;
2222
use Psr\Http\Message\ResponseInterface;
2323
use Psr\Http\Message\StreamInterface;
24+
use enshrined\svgSanitize\Sanitizer;
2425

2526
class ItemController extends Controller
2627
{
@@ -236,7 +237,23 @@ public static function storelogic(Request $request, $id = null): Item
236237
]);
237238

238239
if ($request->hasFile('file')) {
239-
$path = $request->file('file')->store('icons', 'public');
240+
$image = $request->file('file');
241+
$extension = $image->getClientOriginalExtension();
242+
243+
if ($extension === 'svg') {
244+
$sanitizer = new Sanitizer();
245+
$sanitizedSvg = $sanitizer->sanitize(file_get_contents($image->getRealPath()));
246+
247+
// Verify that the sanitization removed malicious content
248+
if (strpos($sanitizedSvg, '<script>') !== false) {
249+
throw ValidationException::withMessages(['file' => 'SVG contains malicious content and cannot be uploaded.']);
250+
}
251+
252+
// Save the sanitized SVG back to the file
253+
file_put_contents($image->getRealPath(), $sanitizedSvg);
254+
}
255+
256+
$path = $image->store('icons', 'public');
240257
$request->merge([
241258
'icon' => $path,
242259
]);
@@ -257,6 +274,16 @@ public static function storelogic(Request $request, $id = null): Item
257274

258275
$contents = file_get_contents($request->input('icon'), false, stream_context_create($options));
259276

277+
if ($extension === 'svg') {
278+
$sanitizer = new Sanitizer();
279+
$contents = $sanitizer->sanitize($contents);
280+
281+
// Verify that the sanitization removed malicious content
282+
if (strpos($contents, '<script>') !== false) {
283+
throw ValidationException::withMessages(['file' => 'SVG contains malicious content and cannot be uploaded.']);
284+
}
285+
}
286+
260287
if (!isImage($contents, $extension)) {
261288
throw ValidationException::withMessages(['file' => 'Icon must be an image.']);
262289
}

‎app/Http/Controllers/SettingsController.php‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Setting;
66
use App\SettingGroup;
77
use Exception;
8+
use enshrined\svgSanitize\Sanitizer;
89
use Illuminate\Contracts\View\View;
910
use Illuminate\Http\RedirectResponse;
1011
use Illuminate\Http\Request;
@@ -68,16 +69,30 @@ public function update(Request $request, int $id): RedirectResponse
6869

6970
if ($setting->type === 'image') {
7071
$validatedData = $request->validate([
71-
'value' => 'image'
72+
'value' => 'image',
7273
]);
7374

7475
if (!$request->hasFile('value')) {
75-
throw new \Exception(
76-
'file_too_big'
77-
);
76+
throw new \Exception('file_too_big');
7877
}
7978

80-
$path = $request->file('value')->store('backgrounds', 'public');
79+
$image = $request->file('value');
80+
$extension = $image->getClientOriginalExtension();
81+
82+
if ($extension === 'svg') {
83+
$sanitizer = new Sanitizer();
84+
$sanitizedSvg = $sanitizer->sanitize(file_get_contents($image->getRealPath()));
85+
86+
// Verify that the sanitization removed malicious content
87+
if (strpos($sanitizedSvg, '<script>') !== false) {
88+
throw new \Exception('SVG contains malicious content and cannot be uploaded.');
89+
}
90+
91+
// Save the sanitized SVG back to the file
92+
file_put_contents($image->getRealPath(), $sanitizedSvg);
93+
}
94+
95+
$path = $image->store('backgrounds', 'public');
8196

8297
if ($path === null) {
8398
throw new \Exception('file_not_stored');
@@ -99,7 +114,7 @@ public function update(Request $request, int $id): RedirectResponse
99114
} catch (Exception $e) {
100115
return redirect($route)
101116
->with([
102-
'errors' => collect([__('app.alert.error.'.$e->getMessage())]),
117+
'errors' => collect([__('app.alert.error.' . $e->getMessage())]),
103118
]);
104119
}
105120
}

‎app/Setting.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Illuminate\Http\Request;
1010
use Illuminate\Session\SessionManager;
1111
use Illuminate\Session\Store;
12+
use enshrined\svgSanitize\Sanitizer;
1213

1314
/**
1415
* App\Setting
@@ -70,9 +71,23 @@ class Setting extends Model
7071

7172
public static function getInput(Request $request): object
7273
{
74+
$image = $request->file('value');
75+
if ($image && $image->getClientOriginalExtension() === 'svg') {
76+
$sanitizer = new Sanitizer();
77+
$sanitizedSvg = $sanitizer->sanitize(file_get_contents($image->getRealPath()));
78+
79+
// Verify that the sanitization removed malicious content
80+
if (strpos($sanitizedSvg, '<script>') !== false) {
81+
throw new \Exception('SVG contains malicious content and cannot be uploaded.');
82+
}
83+
84+
// Save the sanitized SVG back to the file
85+
file_put_contents($image->getRealPath(), $sanitizedSvg);
86+
}
87+
7388
return (object) [
7489
'value' => $request->input('value'),
75-
'image' => $request->file('value'),
90+
'image' => $image,
7691
];
7792
}
7893

@@ -192,7 +207,7 @@ public function getEditValueAttribute()
192207

193208
return $value;
194209
}
195-
210+
196211
public function group(): BelongsTo
197212
{
198213
return $this->belongsTo(\App\SettingGroup::class, 'group_id');

‎composer.json‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,19 @@
99
"type": "project",
1010
"require": {
1111
"php": "^8.2",
12+
"ext-intl": "*",
13+
"ext-json": "*",
14+
"enshrined/svg-sanitize": "^0.21.0",
1215
"graham-campbell/github": "^12.5",
1316
"guzzlehttp/guzzle": "^7.8",
1417
"laravel/framework": "^11.45",
1518
"laravel/tinker": "^2.9",
1619
"laravel/ui": "^4.4",
17-
"nunomaduro/collision": "^8.0",
18-
"symfony/yaml": "^7.0",
19-
"ext-json": "*",
20-
"ext-intl": "*",
2120
"league/flysystem-aws-s3-v3": "^3.0",
21+
"nunomaduro/collision": "^8.0",
22+
"spatie/laravel-html": "^3.11",
2223
"spatie/laravel-ignition": "^2.4",
23-
"spatie/laravel-html": "^3.11"
24+
"symfony/yaml": "^7.0"
2425
},
2526
"require-dev": {
2627
"barryvdh/laravel-ide-helper": "^3.0",

‎composer.lock‎

Lines changed: 48 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/Feature/SVGSanitizerTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
use Tests\TestCase;
3+
use enshrined\svgSanitize\Sanitizer;
4+
5+
class SVGSanitizerTest extends TestCase
6+
{
7+
public function testSvgSanitization()
8+
{
9+
$sanitizer = new Sanitizer();
10+
$maliciousSvg = '<svg><script>alert("XSS")</script></svg>';
11+
$sanitizedSvg = $sanitizer->sanitize($maliciousSvg);
12+
13+
$this->assertStringNotContainsString('<script>', $sanitizedSvg);
14+
}
15+
16+
public function testValidSvgSanitization()
17+
{
18+
$sanitizer = new Sanitizer();
19+
$validSvg = '<svg><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red" /></svg>';
20+
$sanitizedSvg = $sanitizer->sanitize($validSvg);
21+
22+
$this->assertStringContainsString('<circle', $sanitizedSvg);
23+
}
24+
}

‎vendor/composer/autoload_classmap.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
'App\\Application' => $baseDir . '/app/Application.php',
3333
'App\\Console\\Commands\\RegisterApp' => $baseDir . '/app/Console/Commands/RegisterApp.php',
3434
'App\\EnhancedApps' => $baseDir . '/app/EnhancedApps.php',
35+
'App\\Facades\\Form' => $baseDir . '/app/Facades/Form.php',
3536
'App\\Http\\Controllers\\Auth\\ForgotPasswordController' => $baseDir . '/app/Http/Controllers/Auth/ForgotPasswordController.php',
3637
'App\\Http\\Controllers\\Auth\\LoginController' => $baseDir . '/app/Http/Controllers/Auth/LoginController.php',
3738
'App\\Http\\Controllers\\Auth\\RegisterController' => $baseDir . '/app/Http/Controllers/Auth/RegisterController.php',
@@ -57,10 +58,13 @@
5758
'App\\Providers\\RouteServiceProvider' => $baseDir . '/app/Providers/RouteServiceProvider.php',
5859
'App\\Search' => $baseDir . '/app/Search.php',
5960
'App\\SearchInterface' => $baseDir . '/app/SearchInterface.php',
61+
'App\\Services\\CustomFormBuilder' => $baseDir . '/app/Services/CustomFormBuilder.php',
6062
'App\\Setting' => $baseDir . '/app/Setting.php',
6163
'App\\SettingGroup' => $baseDir . '/app/SettingGroup.php',
6264
'App\\SettingUser' => $baseDir . '/app/SettingUser.php',
6365
'App\\SupportedApps' => $baseDir . '/app/SupportedApps.php',
66+
'App\\SupportedApps\\Nzbget\\Nzbget' => $baseDir . '/app/SupportedApps/Nzbget/Nzbget.php',
67+
'App\\SupportedApps\\SABnzbd\\SABnzbd' => $baseDir . '/app/SupportedApps/SABnzbd/SABnzbd.php',
6468
'App\\User' => $baseDir . '/app/User.php',
6569
'Attribute' => $vendorDir . '/symfony/polyfill-php80/Resources/stubs/Attribute.php',
6670
'Aws\\ACMPCA\\ACMPCAClient' => $vendorDir . '/aws/aws-sdk-php/src/ACMPCA/ACMPCAClient.php',
@@ -8103,6 +8107,17 @@
81038107
'Whoops\\Util\\Misc' => $vendorDir . '/filp/whoops/src/Whoops/Util/Misc.php',
81048108
'Whoops\\Util\\SystemFacade' => $vendorDir . '/filp/whoops/src/Whoops/Util/SystemFacade.php',
81058109
'Whoops\\Util\\TemplateHelper' => $vendorDir . '/filp/whoops/src/Whoops/Util/TemplateHelper.php',
8110+
'enshrined\\svgSanitize\\ElementReference\\Resolver' => $vendorDir . '/enshrined/svg-sanitize/src/ElementReference/Resolver.php',
8111+
'enshrined\\svgSanitize\\ElementReference\\Subject' => $vendorDir . '/enshrined/svg-sanitize/src/ElementReference/Subject.php',
8112+
'enshrined\\svgSanitize\\ElementReference\\Usage' => $vendorDir . '/enshrined/svg-sanitize/src/ElementReference/Usage.php',
8113+
'enshrined\\svgSanitize\\Exceptions\\NestingException' => $vendorDir . '/enshrined/svg-sanitize/src/Exceptions/NestingException.php',
8114+
'enshrined\\svgSanitize\\Helper' => $vendorDir . '/enshrined/svg-sanitize/src/Helper.php',
8115+
'enshrined\\svgSanitize\\Sanitizer' => $vendorDir . '/enshrined/svg-sanitize/src/Sanitizer.php',
8116+
'enshrined\\svgSanitize\\data\\AllowedAttributes' => $vendorDir . '/enshrined/svg-sanitize/src/data/AllowedAttributes.php',
8117+
'enshrined\\svgSanitize\\data\\AllowedTags' => $vendorDir . '/enshrined/svg-sanitize/src/data/AllowedTags.php',
8118+
'enshrined\\svgSanitize\\data\\AttributeInterface' => $vendorDir . '/enshrined/svg-sanitize/src/data/AttributeInterface.php',
8119+
'enshrined\\svgSanitize\\data\\TagInterface' => $vendorDir . '/enshrined/svg-sanitize/src/data/TagInterface.php',
8120+
'enshrined\\svgSanitize\\data\\XPath' => $vendorDir . '/enshrined/svg-sanitize/src/data/XPath.php',
81068121
'voku\\helper\\ASCII' => $vendorDir . '/voku/portable-ascii/src/voku/helper/ASCII.php',
81078122
'�' => $vendorDir . '/symfony/cache/Traits/ValueWrapper.php',
81088123
);

0 commit comments

Comments
 (0)