Added XSS check for uploaded SVG files before they get stored

getgrav · Mar 2, 2022 · f19297d · f19297d
1 parent 78b8051
commit f19297d
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 
 1. [](#new)
     * Added support to get image size for SVG vector images [#3533](https://github.com/getgrav/grav/pull/3533)
+    * Added XSS check for uploaded SVG files before they get stored
     * Fixed phpstan issues (All level 2, Framework level 5)
 2. [](#bugfix)
     * Fixed `'mbstring' extension is not loaded` error, use Polyfill instead [#3504](https://github.com/getgrav/grav/pull/3504)

diff --git a/system/src/Grav/Common/Media/Traits/MediaUploadTrait.php b/system/src/Grav/Common/Media/Traits/MediaUploadTrait.php
@@ -100,6 +100,10 @@ public function checkUploadedFile(UploadedFileInterface $uploadedFile, string $f
             'size' => $uploadedFile->getSize(),
         ];
 
+        if ($uploadedFile instanceof FormFlashFile) {
+            $uploadedFile->checkXss();
+        }
+
         return $this->checkFileMetadata($metadata, $filename, $settings);
     }
 

diff --git a/system/src/Grav/Common/Security.php b/system/src/Grav/Common/Security.php
@@ -25,6 +25,22 @@
  */
 class Security
 {
+    /**
+     * @param string $filepath
+     * @param array|null $options
+     * @return string|null
+     */
+    public static function detectXssFromSvgFile(string $filepath, array $options = null): ?string
+    {
+        if (file_exists($filepath) && Grav::instance()['config']->get('security.sanitize_svg')) {
+            $content = file_get_contents($filepath);
+
+            return static::detectXss($content, $options);
+        }
+
+        return null;
+    }
+
     /**
      * Sanitize SVG string for XSS code
      *
@@ -200,7 +216,7 @@ public static function detectXss($string, array $options = null): ?string
         }, $string);
 
         // Clean up entities
-        $string = preg_replace('!(&#[0-9]+)!u', '$1;', $string);
+        $string = preg_replace('!(&#[0-9]+);?!u', '$1;', $string);
 
         // Decode entities
         $string = html_entity_decode($string, ENT_NOQUOTES | ENT_HTML5, 'UTF-8');

diff --git a/system/src/Grav/Framework/Form/FormFlashFile.php b/system/src/Grav/Framework/Form/FormFlashFile.php
@@ -9,6 +9,8 @@
 
 namespace Grav\Framework\Form;
 
+use Grav\Common\Security;
+use Grav\Common\Utils;
 use Grav\Framework\Psr7\Stream;
 use InvalidArgumentException;
 use JsonSerializable;
@@ -182,6 +184,21 @@ public function jsonSerialize()
         return $this->upload;
     }
 
+    /**
+     * @return void
+     */
+    public function checkXss(): void
+    {
+        $tmpFile = $this->getTmpFile();
+        $mime = $this->getClientMediaType();
+        if (Utils::contains($mime, 'svg', false)) {
+            $response = Security::detectXssFromSvgFile($tmpFile);
+            if ($response) {
+                throw new RuntimeException(sprintf('SVG file XSS check failed on %s', $response));
+            }
+        }
+    }
+
     /**
      * @return string|null
      */