BUGFIX: Sanitize uploaded svg files for suspicious contents

neos · Sep 22, 2023 · 0ba8954 · 0ba8954
1 parent 848edb4
commit 0ba8954
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 24 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;
@@ -151,30 +153,21 @@ protected function initialize()
      */
     public function importResource($source, $collectionName = ResourceManager::DEFAULT_PERSISTENT_COLLECTION_NAME, $forcedPersistenceObjectIdentifier = null)
     {
-        $this->initialize();
-        if (!isset($this->collections[$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), 1375196643);
-        }
-
-        /* @var CollectionInterface $collection */
-        $collection = $this->collections[$collectionName];
-
-        try {
-            $resource = $collection->importResource($source);
-            if ($forcedPersistenceObjectIdentifier !== null) {
-                ObjectAccess::setProperty($resource, 'Persistence_Object_Identifier', $forcedPersistenceObjectIdentifier, true);
+        if (is_resource($source)) {
+            $content = stream_get_contents($source);
+            $filename = '';
+            if ($content === false) {
+                throw new Exception('Could not import the content stream as resource.', 1695390671);
             }
-            if (!is_resource($source)) {
-                $pathInfo = UnicodeFunctions::pathinfo($source);
-                $resource->setFilename($pathInfo['basename']);
+        } else {
+            $content = file_get_contents($source);
+            $filename = UnicodeFunctions::pathinfo($source, PATHINFO_FILENAME);
+            if ($content === false) {
+                $this->logger->debug("bar");
+                throw new Exception(sprintf('Could not read the file from "%s" as resource.', $source), 1695390671);
             }
-        } catch (Exception $exception) {
-            throw new Exception(sprintf('Importing a file into the resource collection "%s" failed: %s', $collectionName, $exception->getMessage()), 1375197120, $exception);
         }
-
-        $this->resourceRepository->add($resource);
-        $this->logger->debug(sprintf('Successfully imported file "%s" into the resource collection "%s" (storage: %s, a %s. SHA1: %s)', $source, $collectionName, $collection->getStorage()->getName(), get_class($collection), $resource->getSha1()));
-        return $resource;
+        return $this->importResourceFromContent($content, $filename, $collectionName, $forcedPersistenceObjectIdentifier);
     }
 
     /**
@@ -206,6 +199,8 @@ 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);
         }
 
+        $content = $this->sanitizeImportedFileContent($filename, $content);
+
         /* @var CollectionInterface $collection */
         $collection = $this->collections[$collectionName];
 
@@ -608,6 +603,32 @@ protected function initializeCollections()
         }
     }
 
+    /**
+     * 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 $filename, string $content): string
+    {
+        $typeFromFilename = MediaTypes::getMediaTypeFromFilename($content);
+        $typeFromContent = MediaTypes::getMediaTypeFromFileContent($content);
+        if ($typeFromFilename === 'image/svg+xml' || $typeFromContent === '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) {
+                $content = $sanitizedContent;
+                $this->logger->debug(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/Classes/ResourceManagement/ResourceTypeConverter.php b/Neos.Flow/Classes/ResourceManagement/ResourceTypeConverter.php
@@ -213,7 +213,7 @@ protected function handleFileUploads(array $source, PropertyMappingConfiguration
             $this->convertedResources[$source['tmp_name']] = $resource;
             return $resource;
         } catch (\Exception $exception) {
-            $this->logger->warning('Could not import an uploaded file', ['exception' => $exception] + LogEnvironment::fromMethodName(__METHOD__));
+            $this->logger->warning('Could not import an uploaded file', ['exception' => $exception->getMessage()] + LogEnvironment::fromMethodName(__METHOD__));
             return new FlowError('During import of an uploaded file an error occurred. See log for more details.', 1264517906);
         }
     }
@@ -312,7 +312,7 @@ protected function handleUploadedFile(UploadedFileInterface $source, PropertyMap
             $this->convertedResources[spl_object_hash($source)] = $resource;
             return $resource;
         } catch (\Exception $exception) {
-            $this->logger->warning('Could not import an uploaded file', ['exception' => $exception] + LogEnvironment::fromMethodName(__METHOD__));
+            $this->logger->warning('Could not import an uploaded file', ['exception' => $exception->getMessage()] + LogEnvironment::fromMethodName(__METHOD__));
 
             return new FlowError('During import of an uploaded file an error occurred. See log for more details.', 1264517906);
         }

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/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": "*"