Merge pull request #3249 from dlubitz/revert-sanitize-svg

Revert "BUGFIX: Sanitize uploaded svg files for suspicious contents"

Author: dlubitz

Neos.Flow/Classes/ResourceManagement/ResourceManager.php

@@ -11,12 +11,10 @@
  * 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;
@@ -162,30 +160,14 @@ public function importResource($source, $collectionName = ResourceManager::DEFAU
         $collection = $this->collections[$collectionName];
 
         try {
-            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']);
-            }
+            $resource = $collection->importResource($source);
             if ($forcedPersistenceObjectIdentifier !== null) {
                 ObjectAccess::setProperty($resource, 'Persistence_Object_Identifier', $forcedPersistenceObjectIdentifier, true);
             }
+            if (!is_resource($source)) {
+                $pathInfo = UnicodeFunctions::pathinfo($source);
+                $resource->setFilename($pathInfo['basename']);
+            }
         } catch (Exception $exception) {
             throw new Exception(sprintf('Importing a file into the resource collection "%s" failed: %s', $collectionName, $exception->getMessage()), 1375197120, $exception);
         }
@@ -224,11 +206,6 @@ 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];
 
@@ -631,44 +608,6 @@ 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.

Neos.Flow/composer.json

@@ -51,8 +51,7 @@
 
         "composer/composer": "^2.2.8",
 
-        "egulias/email-validator": "^2.1.17 || ^3.0",
-        "enshrined/svg-sanitize": "^0.16.0"
+        "egulias/email-validator": "^2.1.17 || ^3.0"
     },
     "require-dev": {
         "vimeo/psalm": "~4.30.0",

Neos.Utility.MediaTypes/Classes/MediaTypes.php

@@ -1830,21 +1830,6 @@ 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.
      *

Neos.Utility.MediaTypes/Tests/Unit/MediaTypesTest.php

@@ -68,22 +68,6 @@ 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
      */

composer.json

@@ -32,7 +32,6 @@
         "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": "*"