Skip to content

Commit

Permalink
[CVE-2024-47605] Wrap embeds containing script or style tags in an if…
Browse files Browse the repository at this point in the history
…rame (#11554)

Co-authored-by: Steve Boyd <[email protected]>
  • Loading branch information
GuySartorelli and emteknetnz authored Jan 14, 2025
1 parent a555dad commit 09b5052
Show file tree
Hide file tree
Showing 2 changed files with 308 additions and 1 deletion.
96 changes: 96 additions & 0 deletions src/View/Shortcodes/EmbedShortcodeProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Embed\Http\RequestException;
use Psr\SimpleCache\CacheInterface;
use Psr\SimpleCache\InvalidArgumentException;
use RuntimeException;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\ORM\ArrayList;
Expand All @@ -28,6 +29,31 @@ class EmbedShortcodeProvider implements ShortcodeHandler
{
use Configurable;

/**
* Domains to exclude from sandboxing content in an iframe
* This will also exclude any subdomains
* e.g. if 'example.com' is excluded then 'www.example.com' will also be excluded
* Do not include the protocol in the domain i.e. exclude the leading https://
*/
private static array $domains_excluded_from_sandboxing = [];

/**
* Attributes to add to the iframe when sandboxing
* Note that the 'src' attribute cannot be set via config
* If a style attribute is set via config, width and height values will be overriden by
* any shortcode width and height arguments
*/
private static array $sandboxed_iframe_attributes = [];

/**
* The url of the last extractor used
* This is used instead of adding a new param to an existing method
* which would be backwards incompatible
*
* @internal
*/
private static string $extractorUrl = '';

/**
* Gets the list of shortcodes provided by this handler
*
Expand Down Expand Up @@ -140,6 +166,7 @@ public static function embeddableToHtml(Embeddable $embeddable, array $arguments
return '';
}
$extractor = $embeddable->getExtractor();
EmbedShortcodeProvider::$extractorUrl = (string) $extractor->url;
$type = $embeddable->getType();
if ($type === 'video' || $type === 'rich') {
// Attempt to inherit width (but leave height auto)
Expand Down Expand Up @@ -194,6 +221,7 @@ protected static function videoEmbed($arguments, $content)
]));
}

$content = EmbedShortcodeProvider::sandboxHtml($content, $arguments);
$data = [
'Arguments' => $arguments,
'Attributes' => $attributes,
Expand Down Expand Up @@ -342,4 +370,72 @@ private static function cleanKeySegment(string $str): string
{
return preg_replace('/[^a-zA-Z0-9\-]/', '', $str ?? '');
}

/**
* Wrap potentially dangerous html embeds in an iframe to sandbox them
* Potentially dangerous html embeds would could be those that contain <script> or <style> tags
* or html that contains an on*= attribute
*/
private static function sandboxHtml(string $html, array $arguments)
{
// Do not sandbox if the domain is excluded
if (EmbedShortcodeProvider::domainIsExcludedFromSandboxing()) {
return $html;
}
// Do not sandbox if the html is already an iframe
if (preg_match('#^<iframe[^>]*>#', $html) && preg_match('#</iframe\s*>$#', $html)) {
// Prevent things like <iframe><script>alert(1)</script></iframe>
// and <iframe></iframe><unsafe stuff here/><iframe></iframe>
// If there's more than 2 HTML tags then sandbox it
if (substr_count($html, '<') <= 2) {
return $html;
}
}
// Sandbox the html in an iframe
$style = '';
if (!empty($arguments['width'])) {
$style .= 'width:' . intval($arguments['width']) . 'px;';
}
if (!empty($arguments['height'])) {
$style .= 'height:' . intval($arguments['height']) . 'px;';
}
$attrs = array_merge([
'frameborder' => '0',
], static::config()->get('sandboxed_iframe_attributes'));
$attrs['src'] = 'data:text/html;charset=utf-8,' . rawurlencode($html);
if (array_key_exists('style', $attrs)) {
$attrs['style'] .= ";$style";
$attrs['style'] = ltrim($attrs['style'], ';');
} else {
$attrs['style'] = $style;
}
$html = HTML::createTag('iframe', $attrs);
return $html;
}

/**
* Check if the domain is excluded from sandboxing based on config
*/
private static function domainIsExcludedFromSandboxing(): bool
{
$domain = (string) parse_url(EmbedShortcodeProvider::$extractorUrl, PHP_URL_HOST);
$config = static::config()->get('domains_excluded_from_sandboxing');
foreach ($config as $excluded) {
if (!is_string($excluded)) {
throw new RuntimeException('domains_excluded_from_sandboxing must be an array of strings');
}
$excludedDomain = parse_url($excluded, PHP_URL_HOST);
if (!$excludedDomain) {
// Try adding a protocol so that parse_url can parse it
$excludedDomain = parse_url('http://' . $excluded, PHP_URL_HOST);
}
if (!$excludedDomain) {
throw new RuntimeException('Not a valid domain: ' . $excluded);
}
if (str_ends_with($domain, $excludedDomain)) {
return true;
}
}
return false;
}
}
213 changes: 212 additions & 1 deletion tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

namespace SilverStripe\View\Tests\Shortcodes;

use Embed\Extractor;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Config\Config;
use SilverStripe\View\Parsers\ShortcodeParser;
use SilverStripe\View\Shortcodes\EmbedShortcodeProvider;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\View\Tests\Embed\EmbedUnitTest;
use SilverStripe\View\Embed\EmbedContainer;
use stdClass;
use RuntimeException;

class EmbedShortcodeProviderTest extends EmbedUnitTest
{
Expand Down Expand Up @@ -126,7 +130,7 @@ public function testFlickr()
);
$this->assertEqualIgnoringWhitespace(
<<<EOT
<div style="width:1024px;"><a data-flickr-embed="true" href="https://www.flickr.com/photos/philocycler/32119532132/" title="birdbyPhilocycler,onFlickr"><img src="https://live.staticflickr.com/759/32119532132_50c3f7933f_b.jpg" width="1024" height="742" alt="bird"></a><script asyncsrc="https://embedr.flickr.com/assets/client-code.js" charset="utf-8"></script><p class="caption">Birdy</p></div>
<div style="width:1024px;"><iframe frameborder="0"src="data:text/html;charset=utf-8,%3Ca%20data-flickr-embed%3D%22true%22%20href%3D%22https%3A%2F%2Fwww.flickr.com%2Fphotos%2Fphilocycler%2F32119532132%2F%22%20title%3D%22bird%20by%20Philocycler%2C%20on%20Flickr%22%3E%3Cimg%20src%3D%22https%3A%2F%2Flive.staticflickr.com%2F759%2F32119532132_50c3f7933f_b.jpg%22%20width%3D%221024%22%20height%3D%22742%22%20alt%3D%22bird%22%3E%3C%2Fa%3E%3Cscript%20async%20src%3D%22https%3A%2F%2Fembedr.flickr.com%2Fassets%2Fclient-code.js%22%20charset%3D%22utf-8%22%3E%3C%2Fscript%3E" style="width:1024px;height:742px;"></iframe><p class="caption">Birdy</p></div>
EOT,
$html
);
Expand Down Expand Up @@ -217,4 +221,211 @@ public function testOnlyWhitelistedAttributesAllowed()
$html
);
}

public function provideSandboxHtml(): array
{
return [
'normal' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<divstyle="width:100px;"><iframe frameborder="0"src="data:text/html;'
. 'charset=utf-8,Some%20content"style="width:100px;"></iframe></div>',
],
'normal-with-attrs' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => 'Some content',
'attrs' => [
'frameborder' => '1',
'style' => 'width:200px;height:200px',
'data-something' => 'lorem'
],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe frameborder="1"style="width:200px;'
. 'height:200px;width:100px;" data-something="lorem" src="data:text/html;charset=utf-8,'
. 'Some%20content"></iframe></div>',
],
'excluded' => [
'url' => 'http://example.com/embed',
'excluded' => ['example.com'],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;">Some content</div>',
],
'subdomain-excluded' => [
'url' => 'http://sub.example.com/embed',
'excluded' => ['example.com'],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;">Some content</div>',
],
'config-includes-protocol' => [
'url' => 'http://example.com/embed',
'excluded' => ['http://example.com'],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;">Some content</div>',
],
'config-includes-wrong-protocol' => [
'url' => 'https://example.com/embed',
'excluded' => ['http://example.com'],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;">Some content</div>',
],
'umatched-config' => [
'url' => 'https://example.com/embed',
'excluded' => ['somewhere.com'],
'html' => 'Some content',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe frameborder="0" src="data:text/html;'
. 'charset=utf-8,Some%20content"style="width:100px;"></iframe></div>',
],
'invalid-config' => [
'url' => 'https://example.com/embed',
'excluded' => [123],
'html' => 'Some content',
'attrs' => [],
'exception' => true,
'expected' => '',
],
'iframe' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => '<iframe src="https://example.com/content"></iframe>',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe src="https://example.com/content"></iframe></div>',
],
'iframe-short' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => '<iframe src="https://example.com/content"/>',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe frameborder="0" src="data:text/html;charset=utf-8,'
. '%3Ciframe%20src%3D%22https%3A%2F%2Fexample.com%2Fcontent%22%2F%3E" style="width:100px;">'
. '</iframe></div>',
],
'iframe-whitespace-in-tags' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => '<iframe src="https://example.com/content" ></iframe >',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe src="https://example.com/content"></iframe></div>',
],
'iframe-with-content-inside' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => '<iframe><div>something</div></iframe>',
'attrs' => [],
'exception' => false,
'expected' => '<divstyle="width:100px;"><iframe frameborder="0"src="data:text/html;charset=utf-8,'
. '%3Ciframe%3E%3Cdiv%3Esomething%3C%2Fdiv%3E%3C%2Fiframe%3E"style="width:100px;"></iframe></div>',
],
'closed-iframe' => [
'url' => 'http://example.com/embed',
'excluded' => [],
'html' => '</iframe>',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe frameborder="0"src="data:text/html;'
. 'charset=utf-8,%3C%2Fiframe%3E"style="width:100px;"></iframe></div>',
],
'malicious-iframe-1' => [
'url' => 'https://example.com/embed',
'excluded' => [],
'html' => '<iframe></iframe>bad<iframe></iframe>',
'attrs' => [],
'exception' => false,
'expected' => '<divstyle="width:100px;"><iframe frameborder="0"src="data:text/html;'
. 'charset=utf-8,%3Ciframe%3E%3C%2Fiframe%3Ebad%3Ciframe%3E%3C%2Fiframe%3E"'
. 'style="width:100px;"></iframe></div>',
],
'malicious-iframe-2' => [
'url' => 'https://example.com/embed',
'excluded' => [],
'html' => '<iframe src="http://example.com/thing"></iframe>bad<iframe src="http://example.com/thing"></iframe>',
'attrs' => [],
'exception' => false,
'expected' => '<div style="width:100px;"><iframe frameborder="0"src="data:text/html;'
. 'charset=utf-8,%3Ciframe%20src%3D%22http%3A%2F%2Fexample.com%2Fthing%22%3E%3C%2F'
. 'iframe%3Ebad%3Ciframe%20src%3D%22http%3A%2F%2Fexample.com%2Fthing%22%3E%3C'
. '%2Fiframe%3E"style="width:100px;"></iframe></div>',
],
];
}

/**
* @dataProvider provideSandboxHtml
*/
public function testSandboxHtml(
string $url,
array $excluded,
string $html,
array $attrs,
bool $exception,
string $expected
): void {
if ($exception) {
$this->expectException(RuntimeException::class);
}
$embeddable = $this->getEmbeddable($url, $html);
$attributes = ['width' => 100];
EmbedShortcodeProvider::config()->set('domains_excluded_from_sandboxing', $excluded);
EmbedShortcodeProvider::config()->set('sandboxed_iframe_attributes', $attrs);
$actual = EmbedShortcodeProvider::embeddableToHtml($embeddable, $attributes);
if (!$exception) {
$this->assertEqualIgnoringWhitespace($expected, $actual);
}
}

private function getEmbeddable(string $url, string $html)
{
return new class($url, $html) extends EmbedContainer {
private $_url;
private $_html;
public function __construct($url, $html)
{
$this->_url = $url;
$this->_html = $html;
parent::__construct($url);
}
public function getType()
{
return 'rich';
}
public function getExtractor(): Extractor
{
return new class($this->_url, $this->_html) extends Extractor {
protected $_url;
private $_html;
public function __construct($url, $html)
{
$this->_url = $url;
$this->_html = $html;
}
public function __get($name)
{
$code = new stdClass;
$code->html = $this->_html;
return match ($name) {
'code' => $code,
'url' => $this->_url,
default => null,
};
}
};
}
};
}
}

0 comments on commit 09b5052

Please sign in to comment.