From 168ca00555ce020fa9fb754e27e0a23a58097ed1 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 25 Oct 2022 11:35:05 +1300 Subject: [PATCH] [CVE-2022-38724] Restrict embed shortcode attributes --- .../Shortcodes/EmbedShortcodeProvider.php | 34 +++++++++- .../Shortcodes/EmbedShortcodeProviderTest.php | 64 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/View/Shortcodes/EmbedShortcodeProvider.php b/src/View/Shortcodes/EmbedShortcodeProvider.php index 42f8e947a..7d6f2bbc7 100644 --- a/src/View/Shortcodes/EmbedShortcodeProvider.php +++ b/src/View/Shortcodes/EmbedShortcodeProvider.php @@ -16,6 +16,7 @@ use SilverStripe\View\HTML; use SilverStripe\View\Parsers\ShortcodeHandler; use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\Control\Director; +use SilverStripe\Core\Config\Configurable; use SilverStripe\Dev\Deprecation; use SilverStripe\View\Embed\EmbedContainer; @@ -26,6 +27,23 @@ use SilverStripe\View\Embed\EmbedContainer; */ class EmbedShortcodeProvider implements ShortcodeHandler { + use Configurable; + + /** + * A whitelist of shortcode attributes which are allowed in the resultant markup. + * Note that the tinymce plugin restricts attributes on the client-side separately. + * + * @config + * @deprecated 4.12.0 Removed without equivalent functionality to replace it + */ + private static array $attribute_whitelist = [ + 'url', + 'thumbnail', + 'class', + 'width', + 'height', + 'caption', + ]; /** * Gets the list of shortcodes provided by this handler @@ -207,9 +225,17 @@ class EmbedShortcodeProvider implements ShortcodeHandler } } + $attributes = static::buildAttributeListFromArguments($arguments, ['width', 'height', 'url', 'caption']); + if (array_key_exists('style', $arguments)) { + $attributes->push(ArrayData::create([ + 'Name' => 'style', + 'Value' => Convert::raw2att($arguments['style']), + ])); + } + $data = [ 'Arguments' => $arguments, - 'Attributes' => static::buildAttributeListFromArguments($arguments, ['width', 'height', 'url', 'caption']), + 'Attributes' => $attributes, 'Content' => DBField::create_field('HTMLFragment', $content) ]; @@ -264,6 +290,12 @@ class EmbedShortcodeProvider implements ShortcodeHandler */ private static function buildAttributeListFromArguments(array $arguments, array $exclude = []): ArrayList { + // Clean out any empty arguments and anything not whitelisted + $whitelist = static::config()->get('attribute_whitelist'); + $arguments = array_filter($arguments, function ($value, $key) use ($whitelist) { + return in_array($key, $whitelist) && strlen(trim($value ?? '')); + }, ARRAY_FILTER_USE_BOTH); + $attributes = ArrayList::create(); foreach ($arguments as $key => $value) { if (in_array($key, $exclude ?? [])) { diff --git a/tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php b/tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php index 0fbab6386..a485793a6 100644 --- a/tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php +++ b/tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\View\Tests\Shortcodes; use Psr\SimpleCache\CacheInterface; +use SilverStripe\Core\Config\Config; use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\View\Shortcodes\EmbedShortcodeProvider; use SilverStripe\Dev\SapphireTest; @@ -186,4 +187,67 @@ class EmbedShortcodeProviderTest extends EmbedUnitTest EmbedShortcodeProvider::flushCachedShortcodes($parser, $content); $this->assertFalse($cache->has($key)); } + + public function testOnlyWhitelistedAttributesAllowed() + { + $url = 'https://www.youtube.com/watch?v=dM15HfUYwF0'; + $html = $this->getShortcodeHtml( + $url, + $url, + << + EOT, + << $url, + 'caption' => 'A nice video', + 'width' => 778, + 'height' => 437, + 'data-some-value' => 'my-data', + 'onmouseover' => 'alert(2)', + 'style' => 'background-color:red;', + ], + ); + $this->assertEqualIgnoringWhitespace( + <<

A nice video

+ EOT, + $html + ); + } + + public function testWhitelistIsConfigurable() + { + // Allow new whitelisted attribute + Config::modify()->merge(EmbedShortcodeProvider::class, 'attribute_whitelist', ['data-some-value']); + + $url = 'https://www.youtube.com/watch?v=dM15HfUYwF0'; + $html = $this->getShortcodeHtml( + $url, + $url, + << + EOT, + << $url, + 'caption' => 'A nice video', + 'width' => 779, + 'height' => 437, + 'data-some-value' => 'my-data', + 'onmouseover' => 'alert(2)', + 'style' => 'background-color:red;', + ], + ); + $this->assertEqualIgnoringWhitespace( + <<

A nice video

+ EOT, + $html + ); + } }