Fix bug where it was possible to bypass href URI check with data:appl…

…ication/xhtml+xml URIs (roundcube#6896)
dessert1 · Aug 27, 2019 · 2348899 · 2348899
1 parent 554a20f
commit 2348899
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 7 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -17,9 +17,10 @@ RELEASE 1.3.10
 - Fix bug in converting multi-page Tiff images to Jpeg (#6824)
 - Fix wrong messages order after returning to a multi-folder search result (#6836)
 - Fix PHP 7.4 deprecation: implode() wrong parameter order (#6866)
-- Fix security issue where it was possible to bypass the position:fixed CSS check in received messages (#6898)
+- Fix bug where it was possible to bypass the position:fixed CSS check in received messages (#6898)
 - Fix bug where some strict remote URIs in url() style were unintentionally blocked (#6899)
-- Fix security issue where it was possible to bypass the CSS jail in HTML messages using :root pseudo-class (#6897)
+- Fix bug where it was possible to bypass the CSS jail in HTML messages using :root pseudo-class (#6897)
+- Fix bug where it was possible to bypass href URI check with data:application/xhtml+xml URIs (#6896)
 
 RELEASE 1.3.9
 -------------

diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php
@@ -318,7 +318,7 @@ private function wash_attribs($node)
                     $out = $this->wash_uri($value, true);
                 }
                 else if ($this->is_link_attribute($node->nodeName, $key)) {
-                    if (!preg_match('!^(javascript|vbscript|data:text)!i', $value)
+                    if (!preg_match('!^(javascript|vbscript|data:)!i', $value)
                         && preg_match('!^([a-z][a-z0-9.+-]+:|//|#).+!i', $value)
                     ) {
                         $out = $value;

diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php
@@ -11,19 +11,21 @@ class Framework_Washtml extends PHPUnit_Framework_TestCase
     /**
      * Test the elimination of some XSS vulnerabilities
      */
-    function test_html_xss3()
+    function test_html_xss()
     {
         // #1488850
-        $html = '<p><a href="data:text/html,&lt;script&gt;alert(document.cookie)&lt;/script&gt;">Firefox</a>'
+        $html = '<a href="data:text/html,&lt;script&gt;alert(document.cookie)&lt;/script&gt;">Firefox</a>'
             .'<a href="vbscript:alert(document.cookie)">Internet Explorer</a></p>'
-            .'<p><A href="data:text/html,&lt;script&gt;alert(document.cookie)&lt;/script&gt;">Firefox</a>'
-            .'<A HREF="vbscript:alert(document.cookie)">Internet Explorer</a></p>';
+            .'<A href="data:text/html,&lt;script&gt;alert(document.cookie)&lt;/script&gt;">Firefox</a>'
+            .'<A HREF="vbscript:alert(document.cookie)">Internet Explorer</a>'
+            .'<a href="data:application/xhtml+xml;base64,PGh0bW">CLICK ME</a>'; // #6896
 
         $washer = new rcube_washtml;
         $washed = $washer->wash($html);
 
         $this->assertNotRegExp('/data:text/', $washed, "Remove data:text/html links");
         $this->assertNotRegExp('/vbscript:/', $washed, "Remove vbscript: links");
+        $this->assertNotRegExp('/data:application/', $washed, "Remove data:application links");
     }
 
     /**