Skip to content

Commit 926bff3

Browse files
thorstenthorsten
committed
feat: added sanitizer for SVG content
1 parent db8e2a5 commit 926bff3

File tree

3 files changed

+572
-0
lines changed

3 files changed

+572
-0
lines changed

‎phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ImageController.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use phpMyFAQ\Controller\AbstractController;
2424
use phpMyFAQ\Core\Exception;
2525
use phpMyFAQ\Enums\PermissionType;
26+
use phpMyFAQ\Helper\SvgSanitizer as SvgSanitizer;
2627
use phpMyFAQ\Session\Token;
2728
use phpMyFAQ\Translation;
2829
use Symfony\Component\HttpFoundation\JsonResponse;
@@ -105,6 +106,32 @@ public function upload(Request $request): JsonResponse
105106
$fileName = str_replace(' ', replace: '_', subject: $fileName);
106107
$file->move($uploadDir, $fileName);
107108

109+
$fileExtension = strtolower((string) $file->getClientOriginalExtension());
110+
if ($fileExtension === 'svg') {
111+
$sanitizer = new SvgSanitizer();
112+
$filePath = $uploadDir . $fileName;
113+
114+
if (!$sanitizer->isSafe($filePath)) {
115+
$this->configuration
116+
->getLogger()
117+
->info(sprintf(
118+
'Potentially malicious SVG upload detected: %s by user %d',
119+
$fileName,
120+
$this->currentUser->getUserId(),
121+
));
122+
123+
if (!$sanitizer->sanitize($filePath)) {
124+
if (file_exists($filePath)) {
125+
unlink($filePath);
126+
}
127+
$this->configuration
128+
->getLogger()
129+
->info(sprintf('SVG sanitization failed, file deleted: %s', $fileName));
130+
continue;
131+
}
132+
}
133+
}
134+
108135
// Add to the list of uploaded files
109136
$uploadedFiles[] = $fileName;
110137
}

‎phpmyfaq/src/phpMyFAQ/Helper/SvgSanitizer.php‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
<?php
2+
3+
/**
4+
* SVG Sanitizer (Regex-based) to prevent XSS attacks
5+
* Alternative implementation without ext-dom dependency
6+
*
7+
* This Source Code Form is subject to the terms of the Mozilla Public License,
8+
* v. 2.0. If a copy of the MPL was not distributed with this file, You can
9+
* obtain one at https://mozilla.org/MPL/2.0/.
10+
*
11+
* @package phpMyFAQ
12+
* @author Thorsten Rinne <[email protected]>
13+
* @copyright 2026 phpMyFAQ Team
14+
* @license https://www.mozilla.org/MPL/2.0/ Mozilla Public License Version 2.0
15+
* @link https://www.phpmyfaq.de
16+
* @since 2026-01-15
17+
*/
18+
19+
declare(strict_types=1);
20+
21+
namespace phpMyFAQ\Helper;
22+
23+
class SvgSanitizer
24+
{
25+
/**
26+
* Dangerous patterns to detect and remove
27+
*/
28+
private const array DANGEROUS_PATTERNS = [
29+
// Script tags (any variation)
30+
'/<script\b[^>]*>.*?<\/script>/is',
31+
'/<script\b[^>]*\/>/is',
32+
33+
// Event handlers (onclick, onload, onerror, etc.)
34+
'/\s*on\w+\s*=\s*["\'][^"\']*["\']/i',
35+
'/\s*on\w+\s*=\s*[^"\'\s>][^\s>]*/i',
36+
37+
// ForeignObject tags
38+
'/<foreignObject\b[^>]*>.*?<\/foreignObject>/is',
39+
'/<foreignObject\b[^>]*\/>/is',
40+
41+
// JavaScript URLs in href/xlink:href
42+
'/href\s*=\s*["\']javascript:[^"\']*["\']/i',
43+
'/xlink:href\s*=\s*["\']javascript:[^"\']*["\']/i',
44+
45+
// Data URLs with HTML content
46+
'/href\s*=\s*["\']data:text\/html[^"\']*["\']/i',
47+
'/xlink:href\s*=\s*["\']data:text\/html[^"\']*["\']/i',
48+
49+
// VBScript URLs
50+
'/href\s*=\s*["\']vbscript:[^"\']*["\']/i',
51+
'/xlink:href\s*=\s*["\']vbscript:[^"\']*["\']/i',
52+
53+
// CSS expressions
54+
'/style\s*=\s*["\'][^"\']*expression\s*\([^"\']*["\']/i',
55+
'/style\s*=\s*["\'][^"\']*javascript:[^"\']*["\']/i',
56+
'/style\s*=\s*["\'][^"\']*import[^"\']*["\']/i',
57+
58+
// Embedded XML/HTML entities that could be malicious
59+
'/<!\[CDATA\[.*?<script.*?\]\]>/is',
60+
61+
// XML processing instructions (but allow standard XML declaration)
62+
'/<\?(?!xml)[^?]*\?>/is',
63+
64+
// HTML tags that shouldn't be in SVG
65+
'/<(iframe|embed|object|applet|meta|link|base)\b[^>]*>/i',
66+
];
67+
68+
/**
69+
* Additional dangerous element tags to strip completely
70+
*/
71+
private const array DANGEROUS_ELEMENTS = [
72+
'script',
73+
'foreignObject',
74+
'iframe',
75+
'embed',
76+
'object',
77+
'applet',
78+
'meta',
79+
'link',
80+
'base',
81+
];
82+
83+
/**
84+
* Sanitizes an SVG file by removing potentially dangerous content
85+
*
86+
* @param string $filePath Path to the SVG file
87+
* @return bool True if sanitization was successful, false otherwise
88+
*/
89+
public function sanitize(string $filePath): bool
90+
{
91+
if (!file_exists($filePath)) {
92+
return false;
93+
}
94+
95+
$content = file_get_contents($filePath);
96+
if ($content === false) {
97+
return false;
98+
}
99+
100+
// Check if a file actually contains SVG content
101+
if (!str_contains($content, '<svg')) {
102+
return false;
103+
}
104+
105+
// Remove dangerous patterns
106+
$sanitized = $this->removeDangerousContent($content);
107+
108+
// Verify we still have valid SVG
109+
if (!str_contains($sanitized, '<svg')) {
110+
return false;
111+
}
112+
113+
return file_put_contents($filePath, $sanitized) !== false;
114+
}
115+
116+
/**
117+
* Validates if a file is a safe SVG without dangerous content
118+
*
119+
* @param string $filePath Path to the SVG file
120+
* @return bool True if SVG is safe, false if it contains dangerous content
121+
*/
122+
public function isSafe(string $filePath): bool
123+
{
124+
if (!file_exists($filePath)) {
125+
return false;
126+
}
127+
128+
$content = file_get_contents($filePath);
129+
if ($content === false) {
130+
return false;
131+
}
132+
133+
// Check for dangerous patterns
134+
foreach (self::DANGEROUS_PATTERNS as $pattern) {
135+
if (preg_match($pattern, $content)) {
136+
return false;
137+
}
138+
}
139+
140+
// Check for dangerous element tags
141+
foreach (self::DANGEROUS_ELEMENTS as $element) {
142+
if (preg_match('/<' . preg_quote($element, '/') . '\b/i', $content)) {
143+
return false;
144+
}
145+
}
146+
147+
return true;
148+
}
149+
150+
/**
151+
* Removes dangerous content from SVG string
152+
*
153+
* @param string $content SVG content
154+
* @return string Sanitized content
155+
*/
156+
private function removeDangerousContent(string $content): string
157+
{
158+
$sanitized = $content;
159+
160+
// First pass: Remove dangerous element tags with their content
161+
foreach (self::DANGEROUS_ELEMENTS as $element) {
162+
// Remove opening and closing tags with content
163+
$sanitized = preg_replace(
164+
'/<' . preg_quote($element, '/') . '\b[^>]*>.*?<\/' . preg_quote($element, '/') . '>/is',
165+
'',
166+
$sanitized,
167+
);
168+
169+
// Remove self-closing tags
170+
$sanitized = preg_replace('/<' . preg_quote($element, '/') . '\b[^>]*\/>/is', '', $sanitized);
171+
}
172+
173+
// Second pass: Remove dangerous patterns using regex
174+
foreach (self::DANGEROUS_PATTERNS as $pattern) {
175+
$sanitized = preg_replace($pattern, '', $sanitized);
176+
}
177+
178+
// Third pass: Additional cleanup for remaining event handlers
179+
// This catches edge cases where the attribute might not have quotes
180+
$sanitized = preg_replace('/\s+on\w+\s*=\s*[^\s>]+/i', '', $sanitized);
181+
182+
// Fourth pass: Clean up any remaining javascript: or data:text/html in attributes
183+
$sanitized = preg_replace(
184+
'/(href|xlink:href|src)\s*=\s*(["\'])(?:javascript:|data:text\/html|vbscript:)[^\2]*\2/i',
185+
'',
186+
$sanitized,
187+
);
188+
189+
// Fifth pass: Remove CDATA sections with script content
190+
$sanitized = preg_replace('/<!\[CDATA\[.*?<script.*?\]\]>/is', '', $sanitized);
191+
192+
// Normalize whitespace (optional, for cleaner output)
193+
$sanitized = preg_replace('/\s+/', ' ', $sanitized);
194+
195+
return preg_replace('/>\s+</', '><', $sanitized);
196+
}
197+
198+
/**
199+
* Validates SVG content before saving (strict mode)
200+
* Returns an array of detected issues, empty array if safe
201+
*
202+
* @param string $content SVG content to validate
203+
* @return array<string> List of security issues found
204+
*/
205+
public function detectIssues(string $content): array
206+
{
207+
$issues = [];
208+
209+
foreach (self::DANGEROUS_PATTERNS as $pattern) {
210+
if (preg_match($pattern, $content, $matches)) {
211+
$issues[] = 'Dangerous pattern detected: ' . substr($matches[0], 0, 100);
212+
}
213+
}
214+
215+
foreach (self::DANGEROUS_ELEMENTS as $element) {
216+
if (preg_match('/<' . preg_quote($element, '/') . '\b/i', $content)) {
217+
$issues[] = 'Dangerous element found: ' . $element;
218+
}
219+
}
220+
221+
return $issues;
222+
}
223+
224+
/**
225+
* Alternative: Completely reject SVGs instead of sanitizing
226+
* Use this if you want to be extra cautious
227+
*
228+
* @param string $filePath Path to check
229+
* @return bool True if a file should be rejected
230+
*/
231+
public function shouldReject(string $filePath): bool
232+
{
233+
return !$this->isSafe($filePath);
234+
}
235+
}

0 commit comments

Comments
 (0)