diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ca8efba1..cd6f7f273 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,12 +17,19 @@ ## mm/dd/2022 1. [](#new) - * Added support to get image size for SVG vector images [#3533](https://github.com/getgrav/grav/pull/3533) - * 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) - * 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) + * Added new local Multiavatar (local generation). **This will be default in Grav 1.8** + * 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. [](#improved) + * Moved Accounts out of Experimental section of System configuration +3. [](#bugfix) + * 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 + * Fixed avatar save location when `account://` stream points to custom directory + * Fixed bug in `Utils::url()` when path contains part of root # v1.7.30 ## 02/07/2022 diff --git a/composer.json b/composer.json index 982925098..0ec7697b2 100644 --- a/composer.json +++ b/composer.json @@ -63,7 +63,8 @@ "getgrav/cache": "^2.0", "antoligy/dom-string-iterators": "^1.0", "miljar/php-exif": "^0.6", - "league/climate": "^3.8" + "league/climate": "^3.8", + "multiavatar/multiavatar-php": "^1.0" }, "require-dev": { "codeception/codeception": "^4.1", diff --git a/system/blueprints/config/system.yaml b/system/blueprints/config/system.yaml index f32148f1b..035f9bcd8 100644 --- a/system/blueprints/config/system.yaml +++ b/system/blueprints/config/system.yaml @@ -1757,35 +1757,15 @@ form: validate: type: bool - experimental: + + accounts: type: tab - title: PLUGIN_ADMIN.EXPERIMENTAL + title: PLUGIN_ADMIN.ACCOUNTS fields: - experimental_section: - type: section - title: PLUGIN_ADMIN.EXPERIMENTAL - underline: true - -# flex_pages: -# type: section -# title: Flex Pages -# -# pages.type: -# type: select -# label: PLUGIN_ADMIN.PAGES_TYPE -# highlight: regular -# help: PLUGIN_ADMIN.PAGES_TYPE_HELP -# options: -# regular: PLUGIN_ADMIN.REGULAR -# flex: PLUGIN_ADMIN.FLEX - - pages.type: - type: hidden - flex_accounts: type: section - title: Flex Accounts + title: User Accounts accounts.type: type: select @@ -1804,3 +1784,41 @@ form: options: file: PLUGIN_ADMIN.FILE folder: PLUGIN_ADMIN.FOLDER + + accounts.avatar: + type: select + label: PLUGIN_ADMIN.AVATAR + default: gravatar + help: PLUGIN_ADMIN.AVATAR_HELP + options: + multiavatar: Multiavatar [local] + gravatar: Gravatar [external] + +# experimental: +# type: tab +# title: PLUGIN_ADMIN.EXPERIMENTAL +# +# fields: +# experimental_section: +# type: section +# title: PLUGIN_ADMIN.EXPERIMENTAL +# underline: true +# +# flex_pages: +# type: section +# title: Flex Pages +# +# pages.type: +# type: select +# label: PLUGIN_ADMIN.PAGES_TYPE +# highlight: regular +# help: PLUGIN_ADMIN.PAGES_TYPE_HELP +# options: +# regular: PLUGIN_ADMIN.REGULAR +# flex: PLUGIN_ADMIN.FLEX +# +# pages.type: +# type: hidden + + + diff --git a/system/blueprints/user/account.yaml b/system/blueprints/user/account.yaml index cfe537620..ef5f25b04 100644 --- a/system/blueprints/user/account.yaml +++ b/system/blueprints/user/account.yaml @@ -11,10 +11,21 @@ form: avatar: type: file size: large - destination: 'user://accounts/avatars' + destination: 'account://avatars' multiple: false random_name: true + multiavatar_only: + type: conditional + condition: config.system.accounts.avatar == 'multiavatar' + fields: + avatar_hash: + type: text + label: '' + placeholder: 'e.g. dceaadcfda491f4e45' + description: PLUGIN_ADMIN.AVATAR_HASH + size: large + content: type: section title: PLUGIN_ADMIN.ACCOUNT diff --git a/system/config/system.yaml b/system/config/system.yaml index 82f881398..07afb3c62 100644 --- a/system/config/system.yaml +++ b/system/config/system.yaml @@ -207,6 +207,7 @@ http: accounts: type: regular # EXPERIMENTAL: Account type: regular or flex storage: file # EXPERIMENTAL: Flex storage type: file or folder + avatar: gravatar # Avatar generator [multiavatar|gravatar] flex: cache: diff --git a/system/src/Grav/Common/Flex/Types/Users/UserObject.php b/system/src/Grav/Common/Flex/Types/Users/UserObject.php index 751ceb41e..7a7763e55 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/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 017720ca8..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/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'; } /** diff --git a/system/src/Grav/Common/User/Traits/UserTrait.php b/system/src/Grav/Common/User/Traits/UserTrait.php index 0608caf5e..91c41aa11 100644 --- a/system/src/Grav/Common/User/Traits/UserTrait.php +++ b/system/src/Grav/Common/User/Traits/UserTrait.php @@ -9,12 +9,15 @@ namespace Grav\Common\User\Traits; +use Grav\Common\Filesystem\Folder; use Grav\Common\Grav; use Grav\Common\Page\Medium\ImageMedium; use Grav\Common\Page\Medium\Medium; use Grav\Common\Page\Medium\StaticImageMedium; use Grav\Common\User\Authentication; use Grav\Common\Utils; +use Multiavatar; +use RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator; use function is_array; use function is_string; @@ -175,9 +178,52 @@ trait UserTrait } $email = $this->get('email'); + $avatar_generator = Grav::instance()['config']->get('system.accounts.avatar', 'multiavatar'); + if ($avatar_generator === 'gravatar') { + if (!$email) { + return ''; + } + + $hash = md5(strtolower(trim($email))); + + return 'https://www.gravatar.com/avatar/' . $hash; + } + + $hash = $this->get('avatar_hash'); + if (!$hash) { + $username = $this->get('username'); + $hash = md5(strtolower(trim($email ?? $username))); + } + + return $this->generateMultiavatar($hash); + } + + /** + * @param string $hash + * @return string + */ + protected function generateMultiavatar(string $hash): string + { + /** @var UniformResourceLocator $locator */ + $locator = Grav::instance()['locator']; + + $storage = $locator->findResource('image://multiavatar', true, true); + $avatar_file = "{$storage}/{$hash}.svg"; + + if (!file_exists($storage)) { + Folder::create($storage); + } + + if (!file_exists($avatar_file)) { + $mavatar = new Multiavatar(); + + file_put_contents($avatar_file, $mavatar->generate($hash, null, null)); + } + + $avatar_url = $locator->findResource("image://multiavatar/{$hash}.svg", false, true); + + return Utils::url($avatar_url); - // By default fall back to gravatar image. - return $email ? 'https://www.gravatar.com/avatar/' . md5(strtolower(trim($email))) : ''; } abstract public function get($name, $default = null, $separator = null); diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php index 9ac24c217..145a89356 100644 --- a/system/src/Grav/Common/Utils.php +++ b/system/src/Grav/Common/Utils.php @@ -134,14 +134,13 @@ abstract class Utils $resource = $locator->findResource($input, false); } } else { - $root = $uri->rootUrl(); - - if (static::startsWith($input, $root)) { - $input = static::replaceFirstOccurrence($root, '', $input); + $root = preg_quote($uri->rootUrl(), '#'); + $pattern = '#(' . $root . '$|' . $root . '/)#'; + if (!empty($root) && preg_match($pattern, $input, $matches)) { + $input = static::replaceFirstOccurrence($matches[0], '', $input); } $input = ltrim($input, '/'); - $resource = $input; } 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 */ diff --git a/tests/unit/Grav/Common/UtilsTest.php b/tests/unit/Grav/Common/UtilsTest.php index 32fd84ce1..37e863714 100644 --- a/tests/unit/Grav/Common/UtilsTest.php +++ b/tests/unit/Grav/Common/UtilsTest.php @@ -502,22 +502,26 @@ class UtilsTest extends \Codeception\TestCase\Test self::assertSame('http://testing.dev/subdir/path1/path2/foobar.jpg', Utils::url('/path1/path2/foobar.jpg', true)); self::assertSame('http://testing.dev/subdir/random/path1/path2/foobar.jpg', Utils::url('/random/path1/path2/foobar.jpg', true)); - // Paths including the grav base. + // Absolute Paths including the grav base. self::assertSame('/subdir/', Utils::url('/subdir')); + self::assertSame('/subdir/', Utils::url('/subdir/')); self::assertSame('/subdir/path1', Utils::url('/subdir/path1')); self::assertSame('/subdir/path1/path2', Utils::url('/subdir/path1/path2')); self::assertSame('/subdir/foobar.jpg', Utils::url('/subdir/foobar.jpg')); self::assertSame('/subdir/path1/foobar.jpg', Utils::url('/subdir/path1/foobar.jpg')); - // Relative paths from Grav root with domain. + // Absolute paths from Grav root with domain. self::assertSame('http://testing.dev/subdir/', Utils::url('/subdir', true)); + self::assertSame('http://testing.dev/subdir/', Utils::url('/subdir/', true)); self::assertSame('http://testing.dev/subdir/path1', Utils::url('/subdir/path1', true)); self::assertSame('http://testing.dev/subdir/path1/path2', Utils::url('/subdir/path1/path2', true)); self::assertSame('http://testing.dev/subdir/foobar.jpg', Utils::url('/subdir/foobar.jpg', true)); self::assertSame('http://testing.dev/subdir/path1/foobar.jpg', Utils::url('/subdir/path1/foobar.jpg', true)); // Relative paths from Grav root. + self::assertSame('/subdir/sub', Utils::url('/sub')); self::assertSame('/subdir/subdir', Utils::url('subdir')); + self::assertSame('/subdir/subdir2/sub', Utils::url('/subdir2/sub')); self::assertSame('/subdir/subdir/path1', Utils::url('subdir/path1')); self::assertSame('/subdir/subdir/path1/path2', Utils::url('subdir/path1/path2')); self::assertSame('/subdir/path1', Utils::url('path1'));