tonyair
/
silverstripe-framework
mirror of https://github.com/silverstripe/silverstripe-framework
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #10583 from creative-commoners/pulls/4.11/cve-2022-38724-embed-shortcode 

Restrict embed shortcode attributes
This commit is contained in:
Guy Sartorelli 2022-11-21 13:01:23 +13:00 committed by GitHub
parent bb5b093004 168ca00555
commit b17b29eea1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 97 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/View/Shortcodes/EmbedShortcodeProvider.php
View File

@ -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 ?? [])) {

64
tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php
View File

@ -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
<link rel="alternate" type="application/json+oembed" href="https://www.youtube.com/oembed?format=json&amp;url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3Da2tDOYkFCYo" title="The flying car completes first ever inter-city flight (Official Video)">
EOT,
<<<EOT
{"title":"The flying car completes first ever inter-city flight (Official Video)","author_name":"KleinVision","author_url":"https://www.youtube.com/channel/UCCHAHvcO7KSNmgXVRIJLNkw","type":"video","height":113,"width":200,"version":"1.0","provider_name":"YouTube","provider_url":"https://www.youtube.com/","thumbnail_height":360,"thumbnail_width":480,"thumbnail_url":"https://i.ytimg.com/vi/a2tDOYkFCYo/hqdefault.jpg","html":"\u003ciframe width=\u0022200\u0022 height=\u0022113\u0022 src=\u0022https://www.youtube.com/embed/a2tDOYkFCYo?feature=oembed\u0022 frameborder=\u00220\u0022 allow=\u0022accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture\u0022 allowfullscreen\u003e\u003c/iframe\u003e"}
EOT,
[
'url' => $url,
'caption' => 'A nice video',
'width' => 778,
'height' => 437,
'data-some-value' => 'my-data',
'onmouseover' => 'alert(2)',
'style' => 'background-color:red;',
],
);
$this->assertEqualIgnoringWhitespace(
<<<EOT
<div style="width:778px;"><iframe width="778" height="437" src="https://www.youtube.com/embed/a2tDOYkFCYo?feature=oembed" frameborder="0" allow="accelerometer;autoplay;clipboard-write;encrypted-media;gyroscope;picture-in-picture" allowfullscreen></iframe><p class="caption">A nice video</p></div>
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
<link rel="alternate" type="application/json+oembed" href="https://www.youtube.com/oembed?format=json&amp;url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3Da2tDOYkFCYo" title="The flying car completes first ever inter-city flight (Official Video)">
EOT,
<<<EOT
{"title":"The flying car completes first ever inter-city flight (Official Video)","author_name":"KleinVision","author_url":"https://www.youtube.com/channel/UCCHAHvcO7KSNmgXVRIJLNkw","type":"video","height":113,"width":200,"version":"1.0","provider_name":"YouTube","provider_url":"https://www.youtube.com/","thumbnail_height":360,"thumbnail_width":480,"thumbnail_url":"https://i.ytimg.com/vi/a2tDOYkFCYo/hqdefault.jpg","html":"\u003ciframe width=\u0022200\u0022 height=\u0022113\u0022 src=\u0022https://www.youtube.com/embed/a2tDOYkFCYo?feature=oembed\u0022 frameborder=\u00220\u0022 allow=\u0022accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture\u0022 allowfullscreen\u003e\u003c/iframe\u003e"}
EOT,
[
'url' => $url,
'caption' => 'A nice video',
'width' => 779,
'height' => 437,
'data-some-value' => 'my-data',
'onmouseover' => 'alert(2)',
'style' => 'background-color:red;',
],
);
$this->assertEqualIgnoringWhitespace(
<<<EOT
<div data-some-value="my-data" style="width:779px;"><iframe width="779" height="437" src="https://www.youtube.com/embed/a2tDOYkFCYo?feature=oembed" frameborder="0" allow="accelerometer;autoplay;clipboard-write;encrypted-media;gyroscope;picture-in-picture" allowfullscreen></iframe><p class="caption">A nice video</p></div>
EOT,
$html
);
}
}