From f19297d5f70476e7bedae9f2acef6b43615538b8 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 2 Mar 2022 13:37:51 +0200 Subject: [PATCH] Added XSS check for uploaded SVG files before they get stored --- CHANGELOG.md | 1 + .../Common/Media/Traits/MediaUploadTrait.php | 4 ++++ system/src/Grav/Common/Security.php | 18 +++++++++++++++++- .../src/Grav/Framework/Form/FormFlashFile.php | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4711cfc6..28c2db4557 100644 --- 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 index 36a4503f1b..88591f6aeb 100644 --- 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 index 01ea0f13c8..779e61918a 100644 --- 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 index 6c995993eb..65af544d1b 100644 --- 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 */