Merge pull request #3172 from neos/bugfix/checkUploadedSvgFilesForSus…

…piciousContent BUGFIX: Sanitize uploaded svg files from suspicious content
neos · Oct 12, 2023 · b38faf5 · b38faf5
2 parents 1dec3ac + 9d8daea
commit b38faf5
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 6 deletions.
diff --git a/Neos.Flow/Classes/ResourceManagement/ResourceManager.php b/Neos.Flow/Classes/ResourceManagement/ResourceManager.php
@@ -11,10 +11,12 @@
  * source code.
  */
 
+use enshrined\svgSanitize\Sanitizer;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Log\Utility\LogEnvironment;
 use Neos\Flow\ObjectManagement\ObjectManagerInterface;
 use Neos\Flow\Persistence\PersistenceManagerInterface;
+use Neos\Utility\MediaTypes;
 use Neos\Utility\ObjectAccess;
 use Neos\Flow\ResourceManagement\Storage\StorageInterface;
 use Neos\Flow\ResourceManagement\Storage\WritableStorageInterface;
@@ -160,14 +162,30 @@ public function importResource($source, $collectionName = ResourceManager::DEFAU
         $collection = $this->collections[$collectionName];
 
         try {
-            $resource = $collection->importResource($source);
-            if ($forcedPersistenceObjectIdentifier !== null) {
-                ObjectAccess::setProperty($resource, 'Persistence_Object_Identifier', $forcedPersistenceObjectIdentifier, true);
-            }
-            if (!is_resource($source)) {
+            if (is_resource($source)) {
+                $mediaType = MediaTypes::getMediaTypeFromResource($source);
+                if ($this->isSanitizingRequired($mediaType)) {
+                    $content = stream_get_contents($source);
+                    $resource = $this->importResourceFromContent($content, '', $collectionName, $forcedPersistenceObjectIdentifier);
+                } else {
+                    $resource = $collection->importResource($source);
+                }
+            } else {
+                $resource = fopen($source, 'rb');
+                $mediaType = MediaTypes::getMediaTypeFromResource($resource);
+                fclose($resource);
+                if ($this->isSanitizingRequired($mediaType)) {
+                    $content = file_get_contents($source);
+                    $resource = $this->importResourceFromContent($content, '', $collectionName, $forcedPersistenceObjectIdentifier);
+                } else {
+                    $resource = $collection->importResource($source);
+                }
                 $pathInfo = UnicodeFunctions::pathinfo($source);
                 $resource->setFilename($pathInfo['basename']);
             }
+            if ($forcedPersistenceObjectIdentifier !== null) {
+                ObjectAccess::setProperty($resource, 'Persistence_Object_Identifier', $forcedPersistenceObjectIdentifier, true);
+            }
         } catch (Exception $exception) {
             throw new Exception(sprintf('Importing a file into the resource collection "%s" failed: %s', $collectionName, $exception->getMessage()), 1375197120, $exception);
         }
@@ -206,6 +224,11 @@ public function importResourceFromContent($content, $filename, $collectionName =
             throw new Exception(sprintf('Tried to import a file into the resource collection "%s" but no such collection exists. Please check your settings and the code which triggered the import.', $collectionName), 1380878131);
         }
 
+        $mediaType = MediaTypes::getMediaTypeFromFileContent($content);
+        if ($this->isSanitizingRequired($mediaType)) {
+            $content = $this->sanitizeImportedFileContent($mediaType, $content, $filename);
+        }
+
         /* @var CollectionInterface $collection */
         $collection = $this->collections[$collectionName];
 
@@ -608,6 +631,44 @@ protected function initializeCollections()
         }
     }
 
+    /**
+     * Decide weather the given media-type has to be sanitized
+     * for now this only checks svg file to solve the issue here https://nvd.nist.gov/vuln/detail/CVE-2023-37611
+     *
+     * @todo create a feature from this and allow to register code for sanitizing file content before importing
+     */
+    protected function isSanitizingRequired(string $mediaType): bool
+    {
+        return $mediaType === 'image/svg+xml';
+    }
+
+    /**
+     * Sanitize file content and remove content that is suspicious
+     * for now this only checks svg file to solve the issue here https://nvd.nist.gov/vuln/detail/CVE-2023-37611
+     *
+     * @todo create a feature from this and allow to register code for sanitizing file content before importing
+     */
+    protected function sanitizeImportedFileContent(string $mediaType, string $content, $filename = ''): string
+    {
+        if ($mediaType === 'image/svg+xml') {
+            // @todo: Simplify again when https://github.com/darylldoyle/svg-sanitizer/pull/90 is merged and released.
+            $previousXmlErrorHandling = libxml_use_internal_errors(true);
+            $sanitizer = new Sanitizer();
+            $sanitizedContent = $sanitizer->sanitize($content);
+            libxml_clear_errors();
+            libxml_use_internal_errors($previousXmlErrorHandling);
+            $issues = $sanitizer->getXmlIssues();
+            if ($issues && count($issues) > 0) {
+                if ($sanitizedContent === false) {
+                    throw new Exception('Sanitizing of suspicious file "' . $filename . '" failed during import.', 1695395560);
+                }
+                $content = $sanitizedContent;
+                $this->logger->warning(sprintf('Imported file "%s" contained suspicious content and was sanitized.', $filename), $issues);
+            }
+        }
+        return $content;
+    }
+
     /**
      * Prepare an uploaded file to be imported as resource object. Will check the validity of the file,
      * move it outside of upload folder if open_basedir is enabled and check the filename.

diff --git a/Neos.Flow/composer.json b/Neos.Flow/composer.json
@@ -51,7 +51,8 @@
 
         "composer/composer": "^2.2.8",
 
-        "egulias/email-validator": "^2.1.17 || ^3.0"
+        "egulias/email-validator": "^2.1.17 || ^3.0",
+        "enshrined/svg-sanitize": "^0.16.0"
     },
     "require-dev": {
         "vimeo/psalm": "~4.30.0",

diff --git a/Neos.Utility.MediaTypes/Classes/MediaTypes.php b/Neos.Utility.MediaTypes/Classes/MediaTypes.php
@@ -1830,6 +1830,21 @@ public static function getMediaTypeFromFileContent(string $fileContent): string
         return isset(self::$mediaTypeToFileExtension[$mediaType]) ? $mediaType : 'application/octet-stream';
     }
 
+    /**
+     * Returns a Media Type based on the given resource
+     *
+     * @param resource $resource The resource to determine the media type from
+     * @return string The IANA Internet Media Type
+     */
+    public static function getMediaTypeFromResource($resource): string
+    {
+        if (!is_resource($resource)) {
+            throw new \TypeError('Argument "resource" has to be a resource');
+        }
+        $mediaType = self::trimMediaType(mime_content_type($resource));
+        return isset(self::$mediaTypeToFileExtension[$mediaType]) ? $mediaType : 'application/octet-stream';
+    }
+
     /**
      * Returns the primary filename extension based on the given Media Type.
      *

diff --git a/Neos.Utility.MediaTypes/Tests/Unit/MediaTypesTest.php b/Neos.Utility.MediaTypes/Tests/Unit/MediaTypesTest.php
@@ -68,6 +68,22 @@ public function getMediaTypeFromFileContent(string $filename, string $expectedMe
         self::assertSame($expectedMediaType, MediaTypes::getMediaTypeFromFileContent($fileContent));
     }
 
+    /**
+     * @test
+     * @dataProvider filesAndMediaTypes
+     */
+    public function getMediaTypeFromResource(string $filename, string $expectedMediaType)
+    {
+        $filePath = __DIR__ . '/Fixtures/' . $filename;
+        $resource = is_file($filePath) ? fopen($filePath, 'rb') : fopen('data://text/plain,', 'rb');
+        if ($resource !== false) {
+            self::assertSame($expectedMediaType, MediaTypes::getMediaTypeFromResource($resource));
+            fclose($resource);
+        } else {
+            $this->fail('fixture ' . $filePath . ' could not be read');
+        }
+    }
+
     /**
      * Data Provider
      */

diff --git a/composer.json b/composer.json
@@ -32,6 +32,7 @@
         "neos/composer-plugin": "^2.0",
         "composer/composer": "^2.2.8",
         "egulias/email-validator": "^2.1.17 || ^3.0",
+        "enshrined/svg-sanitize": "^0.16.0",
         "typo3fluid/fluid": "~2.7.0",
         "guzzlehttp/psr7": "^1.8.4",
         "ext-mbstring": "*"