diff options
| -rw-r--r-- | CHANGELOG | 3 | ||||
| -rw-r--r-- | program/lib/Roundcube/rcube_washtml.php | 62 | ||||
| -rw-r--r-- | tests/Framework/Washtml.php | 179 |
3 files changed, 215 insertions, 29 deletions
@@ -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.3.14 -------------- - Security: Fix cross-site scripting (XSS) via HTML messages with malicious svg/namespace diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php index 5d2253b7a..9fcf3c8b8 100644 --- a/program/lib/Roundcube/rcube_washtml.php +++ b/program/lib/Roundcube/rcube_washtml.php @@ -390,7 +390,30 @@ class rcube_washtml 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; } } @@ -400,7 +423,7 @@ class rcube_washtml */ private function is_link_attribute($tag, $attr) { - return ($tag == 'a' || $tag == 'area') && $attr == 'href'; + return $attr === 'href'; } /** @@ -412,6 +435,7 @@ class rcube_washtml || $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 } @@ -425,6 +449,31 @@ class rcube_washtml } /** + * 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 * @@ -458,6 +507,15 @@ class rcube_washtml 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); diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php index 3a4609486..14f625286 100644 --- a/tests/Framework/Washtml.php +++ b/tests/Framework/Washtml.php @@ -288,41 +288,166 @@ class Framework_Washtml extends PHPUnit_Framework_TestCase } /** - * Test SVG cleanup + * Test cases for SVG tests */ - function test_wash_svg2() + function data_wash_svg_tests() { - $svg = '<head xmlns=""><script>alert(document.domain)</script>"><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="" onload="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="" onload="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=""><script>alert(document.domain)</script>"><svg></svg></head>', + '<!-- html ignored --><!-- head ignored --><svg xmlns="http://www.w3.org/1999/xhtml"></svg>' + ], + [ + '<head xmlns="" onload="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="" onload="alert(document.domain)"></svg></p>', + '<p>Hello victim!<svg /></p>' + ], + [ + '<html><p>Hello victim!<svg xmlns="" onload="alert(document.domain)"></svg></p>', + '<!-- html ignored --><!-- body ignored --><p>Hello victim!<svg xmlns="http://www.w3.org/1999/xhtml"></svg></p>' + ], + [ + '<svg xmlns="" onload="alert(document.domain)" />', + '<svg xmlns="" onload="alert(document.domain)" />' + ], + [ + '<html><svg xmlns="" onload="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,<svg id='x' xmlns='http://www.w3.org/2000/svg' " + . "xmlns:xlink='http://www.w3.org/1999/xlink' width='100' height='100'><a xlink:href='javascript:alert(1)'>" + . "<rect x='0' y='0' width='100' height='100' /></a></svg>\"></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="" onload="alert(document.domain)" />'; - $exp = '<svg xmlns="" onload="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="javascript:alert(1)">XSS</a>', + '<!-- html ignored --><body><a x-washed="href">XSS</a></body>' + ], + [ + '<html><a href="j avascript:alert(1)">XSS</a>', + '<!-- html ignored --><body><a x-washed="href">XSS</a></body>' + ], + [ + '<html><a href="j 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"); } /** |