From 3dd0cabeac9835fe64dcb4b68c658b39f1f6be2f Mon Sep 17 00:00:00 2001 From: Djamil Legato Date: Wed, 23 Feb 2022 14:57:36 -0800 Subject: [PATCH 1/5] Fixed entity sanitization for XSS detection --- CHANGELOG.md | 1 + system/src/Grav/Common/Security.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be63eea0e..bb0adfc47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Fixed `'mbstring' extension is not loaded` error, use Polyfill instead [#3504](https://github.com/getgrav/grav/pull/3504) * Fixed new `Utils::pathinfo()` and `Utils::basename()` being too strict for legacy use [#3542](https://github.com/getgrav/grav/issues/3542) * Fixed non-standard video html atributes generated by `{{ media.html() }}` [#3540](https://github.com/getgrav/grav/issues/3540) + * Fixed entity sanitization for XSS detection # v1.7.30 ## 02/07/2022 diff --git a/system/src/Grav/Common/Security.php b/system/src/Grav/Common/Security.php index 017720ca8..01ea0f13c 100644 --- a/system/src/Grav/Common/Security.php +++ b/system/src/Grav/Common/Security.php @@ -200,7 +200,7 @@ class Security }, $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'); From 78b8051627a2404d42641bd63d6a46a6b2fe5f08 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Sat, 26 Feb 2022 19:20:13 +0200 Subject: [PATCH 2/5] Fixed avatar save location when `account://` stream points to custom directory --- CHANGELOG.md | 1 + system/blueprints/user/account.yaml | 2 +- system/src/Grav/Common/Flex/Types/Users/UserObject.php | 2 +- system/src/Grav/Common/User/DataUser/User.php | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb0adfc47..7b4711cfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Fixed new `Utils::pathinfo()` and `Utils::basename()` being too strict for legacy use [#3542](https://github.com/getgrav/grav/issues/3542) * Fixed non-standard video html atributes generated by `{{ media.html() }}` [#3540](https://github.com/getgrav/grav/issues/3540) * Fixed entity sanitization for XSS detection + * Fixed avatar save location when `account://` stream points to custom directory # v1.7.30 ## 02/07/2022 diff --git a/system/blueprints/user/account.yaml b/system/blueprints/user/account.yaml index cfe537620..391c7370a 100644 --- a/system/blueprints/user/account.yaml +++ b/system/blueprints/user/account.yaml @@ -11,7 +11,7 @@ form: avatar: type: file size: large - destination: 'user://accounts/avatars' + destination: 'account://avatars' multiple: false random_name: true diff --git a/system/src/Grav/Common/Flex/Types/Users/UserObject.php b/system/src/Grav/Common/Flex/Types/Users/UserObject.php index d9217917d..9dec9729f 100644 --- a/system/src/Grav/Common/Flex/Types/Users/UserObject.php +++ b/system/src/Grav/Common/Flex/Types/Users/UserObject.php @@ -666,7 +666,7 @@ class UserObject extends FlexObject implements UserInterface, Countable // Check for shared media if (!$folder && !$this->getFlexDirectory()->getMediaFolder()) { $this->_loadMedia = false; - $folder = $this->getBlueprint()->fields()['avatar']['destination'] ?? 'user://accounts/avatars'; + $folder = $this->getBlueprint()->fields()['avatar']['destination'] ?? 'account://avatars'; } return $folder; diff --git a/system/src/Grav/Common/User/DataUser/User.php b/system/src/Grav/Common/User/DataUser/User.php index 71e7f9b35..9cd3904f0 100644 --- a/system/src/Grav/Common/User/DataUser/User.php +++ b/system/src/Grav/Common/User/DataUser/User.php @@ -193,7 +193,7 @@ class User extends Data implements UserInterface */ public function getMediaFolder() { - return $this->blueprints()->fields()['avatar']['destination'] ?? 'user://accounts/avatars'; + return $this->blueprints()->fields()['avatar']['destination'] ?? 'account://avatars'; } /** From f19297d5f70476e7bedae9f2acef6b43615538b8 Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 2 Mar 2022 13:37:51 +0200 Subject: [PATCH 3/5] 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 7b4711cfc..28c2db455 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 36a4503f1..88591f6ae 100644 --- a/system/src/Grav/Common/Media/Traits/MediaUploadTrait.php +++ b/system/src/Grav/Common/Media/Traits/MediaUploadTrait.php @@ -100,6 +100,10 @@ trait MediaUploadTrait '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 01ea0f13c..779e61918 100644 --- a/system/src/Grav/Common/Security.php +++ b/system/src/Grav/Common/Security.php @@ -25,6 +25,22 @@ use function is_string; */ 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 @@ class Security }, $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 6c995993e..65af544d1 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 @@ class FormFlashFile implements UploadedFileInterface, JsonSerializable 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 */ From 34ab8408fa25ec18a5c65067b314c513d6ef7c9b Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 3 Mar 2022 11:21:03 -0700 Subject: [PATCH 4/5] fix for url() function breaking when path contains root --- system/src/Grav/Common/Utils.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php index 64c81a927..d0e0fab15 100644 --- a/system/src/Grav/Common/Utils.php +++ b/system/src/Grav/Common/Utils.php @@ -134,10 +134,10 @@ abstract class Utils $resource = $locator->findResource($input, false); } } else { - $root = $uri->rootUrl(); + $root = $uri->rootUrl() . '/'; if (static::startsWith($input, $root)) { - $input = static::replaceFirstOccurrence($root, '', $input); + $input = static::replaceFirstOccurrence($root, '/', $input); } $input = ltrim($input, '/'); From 879eb275409966a632a65c5c8936d184ecb8fad3 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 3 Mar 2022 11:21:36 -0700 Subject: [PATCH 5/5] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28c2db455..79c680ef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Fixed non-standard video html atributes generated by `{{ media.html() }}` [#3540](https://github.com/getgrav/grav/issues/3540) * Fixed entity sanitization for XSS detection * Fixed avatar save location when `account://` stream points to custom directory + * Fixed bug in `Utils::url()` when path contains root # v1.7.30 ## 02/07/2022