Skip to content

Commit

Permalink
Fix cross-site scripting (XSS) via HTML messages with malicious svg o…
Browse files Browse the repository at this point in the history 
…r math content
  • Loading branch information
@alecpl
alecpl committed Aug 9, 2020
1 parent ce6ebd9 commit 589d360
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 29 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
CHANGELOG Roundcube Webmail
===========================

- Security: Fix cross-site scripting (XSS) via HTML messages with malicious svg content [CVE-2020-16145]
- Security: Fix cross-site scripting (XSS) via HTML messages with malicious math content

RELEASE 1.2.11
--------------
- Security: Fix cross-site scripting (XSS) via HTML messages with malicious svg/namespace
Expand Down
62 changes: 60 additions & 2 deletions program/lib/Roundcube/rcube_washtml.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,30 @@ private function wash_uri($uri, $blocked_source = false)
return $this->config['blocked_src'];
}
}
else if (preg_match('/^data:image.+/i', $uri)) { // RFC2397
else if (preg_match('/^data:image\/([^,]+),(.+)$/i', $uri, $matches)) { // RFC2397
// svg images can be insecure, we'll sanitize them
if (stripos($matches[1], 'svg') !== false) {
$svg = $matches[2];

if (stripos($matches[1], ';base64') !== false) {
$svg = base64_decode($svg);
$type = $matches[1];
}
else {
$type = $matches[1] . ';base64';
}

$washer = new self($this->config);
$svg = $washer->wash($svg);

// Invalid svg content
if (empty($svg)) {
return null;
}

return 'data:image/' . $type . ',' . base64_encode($svg);
}

return $uri;
}
}
Expand All @@ -375,7 +398,7 @@ private function wash_uri($uri, $blocked_source = false)
*/
private function is_link_attribute($tag, $attr)
{
return ($tag == 'a' || $tag == 'area') && $attr == 'href';
return $attr === 'href';
}

/**
Expand All @@ -387,6 +410,7 @@ private function is_image_attribute($tag, $attr)
|| $attr == 'color-profile' // SVG
|| ($attr == 'poster' && $tag == 'video')
|| ($attr == 'src' && preg_match('/^(img|image|source|input|video|audio)$/i', $tag))
|| ($tag == 'use' && $attr == 'href') // SVG
|| ($tag == 'image' && $attr == 'href'); // SVG
}

Expand All @@ -399,6 +423,31 @@ private function is_funciri_attribute($tag, $attr)
'marker-end', 'marker-mid', 'clip-path', 'mask', 'cursor'));
}

/**
* Check if a specified element has an attribute with specified value.
* Do it in case-insensitive manner.
*
* @param DOMElement $node The element
* @param string $attr_name The attribute name
* @param string $attr_value The attribute value to find
*
* @return bool True if the specified attribute exists and has the expected value
*/
private static function attribute_value($node, $attr_name, $attr_value)
{
$attr_name = strtolower($attr_name);

foreach ($node->attributes as $name => $attr) {
if (strtolower($name) === $attr_name) {
if (strtolower($attr_value) === strtolower($attr->nodeValue)) {
return true;
}
}
}

return false;
}

/**
* The main loop that recurse on a node tree.
* It output only allowed tags with allowed attributes and allowed inline styles
Expand Down Expand Up @@ -433,6 +482,15 @@ private function dumpHtml($node, $level = 20)
switch ($node->nodeType) {
case XML_ELEMENT_NODE: //Check element
$tagName = strtolower($node->nodeName);

if (in_array($tagName, array('animate', 'animatecolor', 'set', 'animatetransform'))
&& self::attribute_value($node, 'attributename', 'href')
) {
// Insecure svg tags
$dump .= "<!-- $tagName blocked -->";
break;
}

if ($callback = $this->handlers[$tagName]) {
$dump .= call_user_func($callback, $tagName,
$this->wash_attribs($node), $this->dumpHtml($node, $level), $this);
Expand Down
179 changes: 152 additions & 27 deletions tests/Framework/Washtml.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,41 +271,166 @@ function test_style_wash_svg()
}

/**
* Test SVG cleanup
* Test cases for SVG tests
*/
function test_wash_svg2()
function data_wash_svg_tests()
{
$svg = '<head xmlns="&quot;&gt;&lt;script&gt;alert(document.domain)&lt;/script&gt;"><svg></svg></head>';
$exp = '<!-- html ignored --><!-- head ignored --><svg xmlns="http://www.w3.org/1999/xhtml"></svg>';

$washer = new rcube_washtml;
$washed = $washer->wash($svg);

$this->assertSame($washed, $exp, "SVG content");

$svg = '<head xmlns="&quot; onload=&quot;alert(document.domain)">Hello victim!<svg></svg></head>';
$exp = '<!-- html ignored --><!-- head ignored -->Hello victim!<svg xmlns="http://www.w3.org/1999/xhtml"></svg>';

$washer = new rcube_washtml;
$washed = $washer->wash($svg);

$this->assertSame($washed, $exp, "SVG content");

$svg = '<p>Hello victim!<svg xmlns="&quot; onload=&quot;alert(document.domain)"></svg></p>';
$exp = '<p>Hello victim!<svg /></p>';
$svg1 = "<svg id='x' width='100' height='100'><a xlink:href='javascript:alert(1)'><rect x='0' y='0' width='100' height='100' /></a></svg>";

return [
[
'<head xmlns="&quot;&gt;&lt;script&gt;alert(document.domain)&lt;/script&gt;"><svg></svg></head>',
'<!-- html ignored --><!-- head ignored --><svg xmlns="http://www.w3.org/1999/xhtml"></svg>'
],
[
'<head xmlns="&quot; onload=&quot;alert(document.domain)">Hello victim!<svg></svg></head>',
'<!-- html ignored --><!-- head ignored -->Hello victim!<svg xmlns="http://www.w3.org/1999/xhtml"></svg>'
],
[
'<p>Hello victim!<svg xmlns="&quot; onload=&quot;alert(document.domain)"></svg></p>',
'<p>Hello victim!<svg /></p>'
],
[
'<html><p>Hello victim!<svg xmlns="&quot; onload=&quot;alert(document.domain)"></svg></p>',
'<!-- html ignored --><!-- body ignored --><p>Hello victim!<svg xmlns="http://www.w3.org/1999/xhtml"></svg></p>'
],
[
'<svg xmlns="&quot; onload=&quot;alert(document.domain)" />',
'<svg xmlns="&quot; onload=&quot;alert(document.domain)" />'
],
[
'<html><svg xmlns="&quot; onload=&quot;alert(document.domain)" />',
'<!-- html ignored --><!-- body ignored --><svg xmlns="http://www.w3.org/1999/xhtml"></svg>'
],
[
'<svg><a xlink:href="javascript:alert(1)"><text x="20" y="20">XSS</text></a></svg>',
'<svg><a x-washed="xlink:href"><text x="20" y="20">XSS</text></a></svg>'
],
[
'<html><svg><a xlink:href="javascript:alert(1)"><text x="20" y="20">XSS</text></a></svg>',
'<!-- html ignored --><!-- body ignored --><svg xmlns="http://www.w3.org/1999/xhtml"><a x-washed="xlink:href"><text x="20" y="20">XSS</text></a></svg>'
],
[
'<svg><animate xlink:href="#xss" attributeName="href" values="javascript:alert(1)" />'
. '<a id="xss"><text x="20" y="20">XSS</text></a></svg>',
'<svg><!-- animate blocked --><a id="xss"><text x="20" y="20">XSS</text></a></svg>',
],
[
'<html><svg><animate xlink:href="#xss" attributeName="href" values="javascript:alert(1)" />'
. '<a id="xss"><text x="20" y="20">XSS</text></a></svg>',
'<!-- html ignored --><!-- body ignored --><svg xmlns="http://www.w3.org/1999/xhtml">'
. '<!-- animate blocked --><a id="xss"><text x="20" y="20">XSS</text></a></svg>',
],
[
'<svg><animate xlink:href="#xss" attributeName="href" from="javascript:alert(1)" to="1" />'
. '<a id="xss"><text x="20" y="20">XSS</text></a></svg>',
'<svg><!-- animate blocked --><a id="xss"><text x="20" y="20">XSS</text></a></svg>',
],
[
'<svg><set xlink:href="#xss" attributeName="href" from="?" to="javascript:alert(1)" />'
. '<a id="xss"><text x="20" y="20">XSS</text></a></svg>',
'<svg><!-- set blocked --><a id="xss"><text x="20" y="20">XSS</text></a></svg>',
],
[
'<svg><animate xlink:href="#xss" attributename="href" dur="5s" repeatCount="indefinite" keytimes="0;0;1" values="https://portswigger.net?;javascript:alert(1);0" />'
. '<a id="xss"><text x="20" y="20">XSS</text></a></svg>',
'<svg><!-- animate blocked --><a id="xss"><text x="20" y="20">XSS</text></a></svg>',
],
[
"<svg><use href=\"data:image/svg+xml,&lt;svg id='x' xmlns='http://www.w3.org/2000/svg' "
. "xmlns:xlink='http://www.w3.org/1999/xlink' width='100' height='100'&gt;&lt;a xlink:href='javascript:alert(1)'&gt;"
. "&lt;rect x='0' y='0' width='100' height='100' /&gt;&lt;/a&gt;&lt;/svg&gt;\"></use></svg>",
"<svg><use href=\"data:image/svg+xml;base64,PHN2ZyB4bWxuczp4bGluaz0iaHR0cDovL3d3dy53"
. "My5vcmcvMTk5OS94bGluayIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiBpZD0ie"
. "CIgd2lkdGg9IjEwMCIgaGVpZ2h0PSIxMDAiPjxhIHgtd2FzaGVkPSJ4bGluazpocmVmIj48cmVjdC"
. "B4PSIwIiB5PSIwIiB3aWR0aD0iMTAwIiBoZWlnaHQ9IjEwMCIgLz48L2E+PC9zdmc+\" /></svg>"
],
[
"<svg><use href=\"data:image/svg+xml;base64," . base64_encode($svg1) . "\"></use></svg>",
"<svg><use href=\"data:image/svg+xml;base64,PHN2ZyBpZD0ieCIgd2lkdGg9IjEwMCIgaGVpZ2h"
. "0PSIxMDAiPjxhIHgtd2FzaGVkPSJ4bGluazpocmVmIj48cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0"
. "iMTAwIiBoZWlnaHQ9IjEwMCIgLz48L2E+PC9zdmc+\" /></svg>"
],
[
'<svg><script href="data:text/javascript,alert(1)" /><text x="20" y="20">XSS</text></svg>',
'<svg><!-- script not allowed --><text x="20" y="20">XSS</text></svg>'
],
];
}

/**
* Test SVG cleanup
*
* @dataProvider data_wash_svg_tests
*/
function test_wash_svg_tests($input, $expected)
{
$washer = new rcube_washtml;
$washed = $washer->wash($svg);
$washed = $washer->wash($input);

$this->assertSame($washed, $exp, "SVG content");
$this->assertSame($expected, $washed, "SVG content");
}

$svg = '<svg xmlns="&quot; onload=&quot;alert(document.domain)" />';
$exp = '<svg xmlns="&quot; onload=&quot;alert(document.domain)" />';
/**
* Test cases for various XSS issues
*/
function data_wash_xss_tests()
{
return [
[
'<html><base href="javascript:/a/-alert(1)///////"><a href="../lol/safari.html">test</a>',
'<!-- html ignored --><body><!-- base ignored --><a x-washed="href">test</a></body>'
],
[
'<html><math><x href="javascript:alert(1)">blah</x>',
'<!-- html ignored --><body><math><!-- x ignored -->blah</math></body>'
],
[
'<html><a href="j&#x61vascript:alert(1)">XSS</a>',
'<!-- html ignored --><body><a x-washed="href">XSS</a></body>'
],
[
'<html><a href="&#x6a avascript:alert(1)">XSS</a>',
'<!-- html ignored --><body><a x-washed="href">XSS</a></body>'
],
[
'<html><a href="&#x6a avascript:alert(1)">XSS</a>',
'<!-- html ignored --><body><a x-washed="href">XSS</a></body>'
],
[
'<html><body background="javascript:alert(1)">',
'<!-- html ignored --><body x-washed="background"></body>'
],
[
'<html><math href="javascript:alert(location);"><mi>clickme</mi></math>',
'<!-- html ignored --><body><math x-washed="href"><mi>clickme</mi></math></body>',
],
[
'<html><math><mstyle href="javascript:alert(location);"><mi>clickme</mi></mstyle></math>',
'<!-- html ignored --><body><math><mstyle x-washed="href"><mi>clickme</mi></mstyle></math></body>',
],
[
'<html><math><msubsup href="javascript:alert(location);"><mi>clickme</mi></msubsup></math>',
'<!-- html ignored --><body><math><msubsup x-washed="href"><mi>clickme</mi></msubsup></math></body>',
],
[
'<html><math><ms HREF="javascript:alert(location);">clickme</ms></math>',
'<!-- html ignored --><body><math><ms x-washed="href">clickme</ms></math></body>',
],
];
}

$washer = new rcube_washtml;
$washed = $washer->wash($svg);
/**
* Test various XSS issues
*
* @dataProvider data_wash_xss_tests
*/
function test_wash_xss_tests($input, $expected)
{
$washer = new rcube_washtml(['allow_remote' => true, 'html_elements' => ['body']]);
$washed = $washer->wash($input);

$this->assertSame($washed, $exp, "SVG content");
$this->assertSame($expected, $washed, "XSS issues");
}

/**
Expand Down

0 comments on commit 589d360

Please sign in to comment.