[TASK] Introduce SVG Sanitizer

This change introduces behavior of extension `t3g/svg-sanitizer` into the TYPO3 core. Sanitizing SVG data is actually done by external package `enshrined/svg-sanitize` by Daryll Doyle. The following aspects are introduced: + handle `GeneralUtility::upload_copy_move` invocations + handle FAL action events `file-add`, `file-replace`, `set-content` + provide upgrade wizard, sanitizing all SVG files in storages that are using `LocalDriver` Custom usage: ``` $sanitizer = new \TYPO3\CMS\Core\Resource\Security\SvgSanitizer(); $sanitizer->sanitizeFile($sourcePath, $targetPath); $svg = $sanitizer->sanitizeContent($svg); ``` Basically this change enforces following public service announcements concerning SVG files, to enhance these security aspects per default: + https://typo3.org/security/advisory/typo3-psa-2020-003 + https://typo3.org/security/advisory/typo3-psa-2019-010 Resolves: #94492 Releases: master, 10.4, 9.5 Change-Id: I42c206190d8a335ebaf77b7e5d57b383e3bcbae1 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69817 Tested-by: core-ci <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>
TYPO3 · Jul 13, 2021 · 45b389d · 45b389d
1 parent a06a256
commit 45b389d
Show file tree

Hide file tree

Showing 53 changed files with 1,804 additions and 2 deletions.
diff --git a/composer.json b/composer.json
@@ -44,6 +44,7 @@
 		"doctrine/instantiator": "^1.1",
 		"doctrine/lexer": "^1.0",
 		"egulias/email-validator": "^2.1",
+		"enshrined/svg-sanitize": "^0.14.0",
 		"guzzlehttp/guzzle": "^6.3.0",
 		"guzzlehttp/psr7": "^1.4.0",
 		"nikic/php-parser": "^4.10.4",

diff --git a/composer.lock b/composer.lock
diff --git a/typo3/sysext/core/Classes/Resource/Security/SvgEventListener.php b/typo3/sysext/core/Classes/Resource/Security/SvgEventListener.php
@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Resource\Security;
+
+use TYPO3\CMS\Core\Resource\Event\AfterFileContentsSetEvent;
+use TYPO3\CMS\Core\Resource\Event\BeforeFileAddedEvent;
+use TYPO3\CMS\Core\Resource\Event\BeforeFileReplacedEvent;
+
+class SvgEventListener
+{
+    /**
+     * @var SvgSanitizer
+     */
+    protected $sanitizer;
+
+    /**
+     * @var SvgTypeCheck
+     */
+    protected $typeCheck;
+
+    public function __construct(SvgSanitizer $sanitizer, SvgTypeCheck $typeCheck)
+    {
+        $this->sanitizer = $sanitizer;
+        $this->typeCheck = $typeCheck;
+    }
+
+    public function beforeFileAdded(BeforeFileAddedEvent $event): void
+    {
+        $filePath = $event->getSourceFilePath();
+        if ($this->typeCheck->forFilePath($filePath)) {
+            $this->sanitizer->sanitizeFile($filePath);
+        }
+    }
+
+    public function beforeFileReplaced(BeforeFileReplacedEvent $event): void
+    {
+        $filePath = $event->getLocalFilePath();
+        if ($this->typeCheck->forFilePath($filePath)) {
+            $this->sanitizer->sanitizeFile($filePath);
+        }
+    }
+
+    public function afterFileContentsSet(AfterFileContentsSetEvent $event): void
+    {
+        $file = $event->getFile();
+        if (!$this->typeCheck->forResource($file)) {
+            return;
+        }
+        $content = $event->getContent();
+        $sanitizedContent = $this->sanitizer->sanitizeContent($content);
+        // cave: setting content will trigger calling this handler again
+        // (having custom-flags on `FileInterface` would allow to mark it as "processed")
+        if ($sanitizedContent !== $content) {
+            $file->setContents($sanitizedContent);
+        }
+    }
+}
diff --git a/typo3/sysext/core/Classes/Resource/Security/SvgHookHandler.php b/typo3/sysext/core/Classes/Resource/Security/SvgHookHandler.php
@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Resource\Security;
+
+class SvgHookHandler
+{
+    /**
+     * @var SvgSanitizer
+     */
+    protected $sanitizer;
+
+    /**
+     * @var SvgTypeCheck
+     */
+    protected $typeCheck;
+
+    public function __construct(SvgSanitizer $sanitizer, SvgTypeCheck $typeCheck)
+    {
+        $this->sanitizer = $sanitizer;
+        $this->typeCheck = $typeCheck;
+    }
+
+    /**
+     * @param array $parameters
+     */
+    public function processMoveUploadedFile(array $parameters)
+    {
+        $filePath = $parameters['source'] ?? null;
+        if ($filePath !== null && $this->typeCheck->forFilePath($filePath)) {
+            $this->sanitizer->sanitizeFile($filePath);
+        }
+    }
+}
diff --git a/typo3/sysext/core/Classes/Resource/Security/SvgSanitizer.php b/typo3/sysext/core/Classes/Resource/Security/SvgSanitizer.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Resource\Security;
+
+use enshrined\svgSanitize\Sanitizer;
+
+class SvgSanitizer
+{
+    /**
+     * @param string $sourcePath
+     * @param string|null $targetPath
+     * @throws \BadFunctionCallException
+     */
+    public function sanitizeFile(string $sourcePath, string $targetPath = null): void
+    {
+        if ($targetPath === null) {
+            $targetPath = $sourcePath;
+        }
+        $svg = file_get_contents($sourcePath);
+        if (!is_string($svg)) {
+            return;
+        }
+        $sanitizedSvg = $this->sanitizeContent($svg);
+        if ($sanitizedSvg !== $svg) {
+            file_put_contents($targetPath, $sanitizedSvg);
+        }
+    }
+
+    /**
+     * @param string $svg
+     *
+     * @return string
+     * @throws \BadFunctionCallException
+     */
+    public function sanitizeContent(string $svg): string
+    {
+        $sanitizer = new Sanitizer();
+        $sanitizer->removeRemoteReferences(true);
+        return $sanitizer->sanitize($svg) ?: '';
+    }
+}
diff --git a/typo3/sysext/core/Classes/Resource/Security/SvgTypeCheck.php b/typo3/sysext/core/Classes/Resource/Security/SvgTypeCheck.php
@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Resource\Security;
+
+use TYPO3\CMS\Core\Resource\FileInterface;
+use TYPO3\CMS\Core\Resource\MimeTypeDetector;
+use TYPO3\CMS\Core\Type\File\FileInfo;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
+class SvgTypeCheck
+{
+    protected const MIME_TYPES = ['image/svg', 'image/svg+xml', 'application/svg', 'application/svg+xml'];
+
+    /**
+     * @var MimeTypeDetector
+     */
+    protected $mimeTypeDetector;
+
+    /**
+     * @var string[]
+     */
+    protected $fileExtensions;
+
+    public function __construct(MimeTypeDetector $mimeTypeDetector)
+    {
+        $this->mimeTypeDetector = $mimeTypeDetector;
+        $this->fileExtensions = $this->resolveFileExtensions();
+    }
+
+    public function forFilePath(string $filePath): bool
+    {
+        $fileInfo = GeneralUtility::makeInstance(FileInfo::class, $filePath);
+        $fileExtension = $fileInfo->getExtension();
+        $mimeType = $fileInfo->getMimeType();
+        return in_array($fileExtension, $this->fileExtensions, true)
+            || in_array($mimeType, self::MIME_TYPES, true);
+    }
+
+    public function forResource(FileInterface $file): bool
+    {
+        $fileExtension = $file->getExtension();
+        $mimeType = $file->getMimeType();
+        return in_array($fileExtension, $this->fileExtensions, true)
+            || in_array($mimeType, self::MIME_TYPES, true);
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function resolveFileExtensions(): array
+    {
+        $fileExtensions = array_map(
+            function (string $mimeType): array {
+                return $this->mimeTypeDetector->getFileExtensionsForMimeType($mimeType);
+            },
+            self::MIME_TYPES
+        );
+        $fileExtensions = array_filter($fileExtensions);
+        return count($fileExtensions) > 0 ? array_unique(array_merge(...$fileExtensions)) : [];
+    }
+}
diff --git a/typo3/sysext/core/Configuration/Services.yaml b/typo3/sysext/core/Configuration/Services.yaml
@@ -342,6 +342,27 @@ services:
         event: TYPO3\CMS\Core\Resource\Event\AfterFileDeletedEvent
 
 
+  TYPO3\CMS\Core\Resource\Security\SvgEventListener:
+    tags:
+      - name: event.listener
+        identifier: 'svg-resource-storage-listener-before-file-added'
+        method: 'beforeFileAdded'
+        event: TYPO3\CMS\Core\Resource\Event\BeforeFileAddedEvent
+      - name: event.listener
+        identifier: 'svg-resource-storage-listener-before-file-replaced'
+        method: 'beforeFileReplaced'
+        event: TYPO3\CMS\Core\Resource\Event\BeforeFileReplacedEvent
+      - name: event.listener
+        identifier: 'svg-resource-storage-listener-after-file-content-set'
+        method: 'afterFileContentsSet'
+        event: TYPO3\CMS\Core\Resource\Event\AfterFileContentsSetEvent
+
+  TYPO3\CMS\Core\Resource\Security\SvgHookHandler:
+    public: true
+
+  TYPO3\CMS\Core\Resource\Security\SvgTypeCheck:
+    public: true
+
   # Core caches, cache.core and cache.assets are injected as early
   # entries in TYPO3\CMS\Core\Core\Bootstrap and therefore omitted here
   cache.hash:

diff --git a/...xt/core/Documentation/Changelog/9.5.x/Important-94492-IntroduceSVGSanitizer.rst b/...xt/core/Documentation/Changelog/9.5.x/Important-94492-IntroduceSVGSanitizer.rst
@@ -0,0 +1,39 @@
+.. include:: ../../Includes.txt
+
+===========================================
+Important: #94492 - Introduce SVG Sanitizer
+===========================================
+
+See :issue:`94492`
+
+Description
+===========
+
+SVG sanitization behavior of extension [`t3g/svg-sanitizer`](https://packagist.org/packages/t3g/svg-sanitizer)
+has been introduced into TYPO3 core. Actual processing is done by low-level sanitization package
+[`enshrined/svg-sanitize`](https://packagist.org/packages/enshrined/svg-sanitize) by Daryll Doyle.
+
+Introduced aspects
+------------------
+
+* handle :php:`GeneralUtility::upload_copy_move` invocations
+* handle FAL action events `file-add`, `file-replace`, `set-content`
+* provide upgrade wizard, sanitizing all SVG files in storages that
+  are using :php:`\TYPO3\CMS\Core\Resource\Driver\LocalDriver`
+
+Custom usage
+------------
+
+.. code-block:: php
+
+   $sanitizer = new \TYPO3\CMS\Core\Resource\Security\SvgSanitizer();
+   $sanitizer->sanitizeFile($sourcePath, $targetPath);
+   $svg = $sanitizer->sanitizeContent($svg);
+
+Basically this change enforces following public service announcements
+concerning SVG files, to enhance these security aspects per default:
+
+* [TYPO3-PSA-2020-003: Mitigation of Cross-Site Scripting Vulnerabilities in File Upload Handling](https://typo3.org/security/advisory/typo3-psa-2020-003)
+* [TYPO3-PSA-2019-010: Cross-Site Scripting Vulnerabilities in File Upload Handling](https://typo3.org/security/advisory/typo3-psa-2019-010)
+
+.. index:: Backend, FAL, Frontend, ext:core