Fix cross-site scripting (XSS) via HTML messages with malicious svg/n…

…amespace Credits to SSD Secure Disclosure (https://ssd-disclosure.com/)
roundcube · Jul 3, 2020 · 3e8832d · 3e8832d
1 parent 2531eb7
commit 3e8832d
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 2 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -7,6 +7,7 @@ CHANGELOG Roundcube Webmail
 - Elastic: Fix context menu (paste) on the recipient input (#7431)
 - Fix problem with forwarding inline images attached to messages with no HTML part (#7414)
 - Fix problem with handling attached images with same name when using database_attachments/redundant_attachments (#7455)
+- Security: Fix cross-site scripting (XSS) via HTML messages with malicious svg/namespace
 
 RELEASE 1.4.6
 -------------

diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php
@@ -521,7 +521,10 @@ private function dumpHtml($node, $level = 20)
                         $xpath = new DOMXPath($node->ownerDocument);
                         foreach ($xpath->query('namespace::*') as $ns) {
                             if ($ns->nodeName != 'xmlns:xml') {
-                                $dump .= ' ' . $ns->nodeName . '="' . $ns->nodeValue . '"';
+                                $dump .= sprintf(' %s="%s"',
+                                    $ns->nodeName,
+                                    htmlspecialchars($ns->nodeValue, ENT_QUOTES, $this->config['charset'])
+                                );
                             }
                         }
                     }
@@ -588,7 +591,7 @@ public function wash($html)
         $this->max_nesting_level = (int) @ini_get('xdebug.max_nesting_level');
 
         // SVG need to be parsed as XML
-        $this->is_xml = stripos($html, '<html') === false && stripos($html, '<svg') !== false;
+        $this->is_xml = !preg_match('/<(html|head|body)/i', $html) && stripos($html, '<svg') !== false;
         $method       = $this->is_xml ? 'loadXML' : 'loadHTML';
 
         // DOMDocument does not support HTML5, try Masterminds parser if available

diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php
@@ -315,6 +315,44 @@ function test_wash_svg()
         $this->assertSame($washed, $exp, "SVG content");
     }
 
+    /**
+     * Test SVG cleanup
+     */
+    function test_wash_svg2()
+    {
+        $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>';
+
+        $washer = new rcube_washtml;
+        $washed = $washer->wash($svg);
+
+        $this->assertSame($washed, $exp, "SVG content");
+
+        $svg = '<svg xmlns="&quot; onload=&quot;alert(document.domain)" />';
+        $exp = '<svg xmlns="&quot; onload=&quot;alert(document.domain)" />';
+
+        $washer = new rcube_washtml;
+        $washed = $washer->wash($svg);
+
+        $this->assertSame($washed, $exp, "SVG content");
+    }
+
     /**
      * Test position:fixed cleanup - (#5264)
      */