CVE-2016-4068: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in Roundcube Webmail before 1.0.9 and 1.1.x before 1.1.5 allows remote attackers to inject arbitrary web script or HTML via a crafted SVG, a different vulnerability than CVE-2015-8864.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2017-04-13T14:59Z
Repository: https://github.com/roundcube/roundcubemail
Language: Other
Files changed: CHANGELOG, program/steps/mail/get.inc
Commit date: 2016-01-06 14:10:05 +0100
Commit message: Fix XSS issue in SVG images handling (#1490625)

Diff: diff --git a/CHANGELOG b/CHANGELOG
index 4a66c5138d..dadf1f9845 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,10 +8,11 @@ CHANGELOG Roundcube Webmail
 - Fix path traversal vulnerability in setting a skin (#1490620)
 - Fix so drag-n-drop of text (e.g. recipient addresses) on compose page actually works (#1490619)
 - Fix .htaccess rewrite rules to not block .well-known URIs (#1490615)
 - Fix mail view scaling on iOS (#1490551)
 - Fix PHP7 warning "session_start(): Session callback expects true/false return value" (#1490624)
+- Fix XSS issue in SVG images handling (#1490625)
 
 RELEASE 1.2-beta
 ----------------
 - Update TinyMCE to version 4.2
 - Remove backward compatibility "layer" of bc.php (#1490534)
diff --git a/program/steps/mail/get.inc b/program/steps/mail/get.inc
index 9806303f8e..96cdd77179 100644
--- a/program/steps/mail/get.inc
+++ b/program/steps/mail/get.inc
@@ -91,10 +91,15 @@ else if ($_GET['_thumb']) {
                 $image = new rcube_image($orig_name);
                 if ($imgtype = $image->resize($thumbnail_size, $cache_file, true)) {
                     $mimetype = 'image/' . $imgtype;
                     unlink($orig_name);
                 }
+                else if (stripos($mimetype, 'image/svg') === 0) {
+                    $content = rcmail_svg_filter(file_get_contents($orig_name));
+                    file_put_contents($cache_file, $content);
+                    unlink($orig_name);
+                }
                 else {
                     rename($orig_name, $cache_file);
                 }
             }
         }
@@ -327,45 +332,21 @@ else if (strlen($part_id)) {
                     $saved = $MESSAGE->get_part_body($part->mime_id, false, 0, $fd);
                     fclose($fd);
                 }
 
                 // convert image to jpeg and send it to the browser
-                if ($saved) {
+                if ($sent = $saved) {
                     $image = new rcube_image($file_path);
                     if ($image->convert(rcube_image::TYPE_JPG, $file_path)) {
                         header("Content-Length: " . filesize($file_path));
                         readfile($file_path);
                     }
                     unlink($file_path);
                 }
             }
-            // do content filtering to avoid XSS through fake images
-            else if (!empty($_REQUEST['_embed']) && $browser->ie && $browser->ver <= 8) {
-                if ($body) {
-                    echo preg_match('/<(script|iframe|object)/i', $body) ? '' : $body;
-                    $sent = true;
-                }
-                else if ($part->size) {
-                    $stdout = fopen('php://output', 'w');
-                    stream_filter_register('rcube_content', 'rcube_content_filter') or die('Failed to register content filter');
-                    stream_filter_append($stdout, 'rcube_content');
-                    $sent = $MESSAGE->get_part_body($part->mime_id, true, 0, $stdout);
-                }
-            }
-            // send part as-it-is
             else {
-                if ($body && empty($plugin['download'])) {
-                    header("Content-Length: " . strlen($body));
-                    echo $body;
-                    $sent = true;
-                }
-                else if ($part->size) {
-                    // Don't be tempted to set Content-Length to $part->d_parameters['size'] (#1490482)
-                    // RFC2183 says "The size parameter indicates an approximate size"
-
-                    $sent = $MESSAGE->get_part_body($part->mime_id, false, 0, -1);
-                }
+                $sent = rcmail_message_part_output($body, $part, $mimetype, $plugin['download']);
             }
 
             // check connection status
             if ($part->size && !$sent) {
                 check_storage_status();
@@ -473,5 +454,73 @@ function rcmail_message_part_frame($attrib)
 
     $RCMAIL->output->add_gui_object('messagepartframe', $attrib['id']);
 
     return html::iframe($attrib);
 }
+
+/**
+ * Output attachment body with content filtering
+ */
+function rcmail_message_part_output($body, $part, $mimetype, $download)
+{
+    global $MESSAGE, $RCMAIL;
+
+    if (!$part->size && !$body) {
+        return false;
+    }
+
+    $browser = $RCMAIL->output->browser;
+    $secure  = stripos($mimetype, 'image/') === false || $download;
+
+    // Remove <script> in SVG images
+    if (!$secure && stripos($mimetype, 'image/svg') === 0) {
+        if (!$body) {
+            $body = $MESSAGE->get_part_body($part->mime_id, false);
+            if (empty($body)) {
+                return false;
+            }
+        }
+
+        echo rcmail_svg_filter($body);
+        return true;
+    }
+
+    // Remove dangerous content in images for older IE (to be removed)
+    if (!$secure && $browser->ie && $browser->ver <= 8) {
+        if ($body) {
+            echo preg_match('/<(script|iframe|object)/i', $body) ? '' : $body;
+            return true;
+        }
+        else {
+            $stdout = fopen('php://output', 'w');
+            stream_filter_register('rcube_content', 'rcube_content_filter') or die('Failed to register content filter');
+            stream_filter_append($stdout, 'rcube_content');
+            return $MESSAGE->get_part_body($part->mime_id, true, 0, $stdout);
+        }
+    }
+
+    if ($body && !$download) {
+        header("Content-Length: " . strlen($body));
+        echo $body;
+        return true;
+    }
+
+    // Don't be tempted to set Content-Length to $part->d_parameters['size'] (#1490482)
+    // RFC2183 says "The size parameter indicates an approximate size"
+
+    return $MESSAGE->get_part_body($part->mime_id, false, 0, -1);
+}
+
+/**
+ * Remove <script> in SVG images
+ */
+function rcmail_svg_filter($body)
+{
+    $dom = new DOMDocument;
+    $dom->loadXML($body);
+
+    foreach ($dom->getElementsByTagName('script') as $node) {
+        $node->parentNode->removeChild($node);
+    }
+
+    return $dom->saveXML() ?: '';
+}

Vulnerable code:                 if ($saved) {
            // do content filtering to avoid XSS through fake images
            else if (!empty($_REQUEST['_embed']) && $browser->ie && $browser->ver <= 8) {
                if ($body) {
                    echo preg_match('/<(script|iframe|object)/i', $body) ? '' : $body;
                    $sent = true;
                }
                else if ($part->size) {
                    $stdout = fopen('php://output', 'w');
                    stream_filter_register('rcube_content', 'rcube_content_filter') or die('Failed to register content filter');
                    stream_filter_append($stdout, 'rcube_content');
                    $sent = $MESSAGE->get_part_body($part->mime_id, true, 0, $stdout);
                }
            }
            // send part as-it-is
                if ($body && empty($plugin['download'])) {
                    header("Content-Length: " . strlen($body));
                    echo $body;
                    $sent = true;
                }
                else if ($part->size) {
                    // Don't be tempted to set Content-Length to $part->d_parameters['size'] (#1490482)
                    // RFC2183 says "The size parameter indicates an approximate size"

                    $sent = $MESSAGE->get_part_body($part->mime_id, false, 0, -1);
                }
Fixed code: - Fix XSS issue in SVG images handling (#1490625)
                else if (stripos($mimetype, 'image/svg') === 0) {
                    $content = rcmail_svg_filter(file_get_contents($orig_name));
                    file_put_contents($cache_file, $content);
                    unlink($orig_name);
                }
                if ($sent = $saved) {
                $sent = rcmail_message_part_output($body, $part, $mimetype, $plugin['download']);

/**
 * Output attachment body with content filtering
 */
function rcmail_message_part_output($body, $part, $mimetype, $download)
{
    global $MESSAGE, $RCMAIL;

    if (!$part->size && !$body) {
        return false;
    }

    $browser = $RCMAIL->output->browser;
    $secure  = stripos($mimetype, 'image/') === false || $download;

    // Remove <script> in SVG images
    if (!$secure && stripos($mimetype, 'image/svg') === 0) {
        if (!$body) {
            $body = $MESSAGE->get_part_body($part->mime_id, false);
            if (empty($body)) {
                return false;
            }
        }

        echo rcmail_svg_filter($body);
        return true;
    }

    // Remove dangerous content in images for older IE (to be removed)
    if (!$secure && $browser->ie && $browser->ver <= 8) {
        if ($body) {
            echo preg_match('/<(script|iframe|object)/i', $body) ? '' : $body;
            return true;
        }
        else {
            $stdout = fopen('php://output', 'w');
            stream_filter_register('rcube_content', 'rcube_content_filter') or die('Failed to register content filter');
            stream_filter_append($stdout, 'rcube_content');
            return $MESSAGE->get_part_body($part->mime_id, true, 0, $stdout);
        }
    }

    if ($body && !$download) {
        header("Content-Length: " . strlen($body));
        echo $body;
        return true;
    }

    // Don't be tempted to set Content-Length to $part->d_parameters['size'] (#1490482)
    // RFC2183 says "The size parameter indicates an approximate size"

    return $MESSAGE->get_part_body($part->mime_id, false, 0, -1);
}

/**
 * Remove <script> in SVG images
 */
function rcmail_svg_filter($body)
{
    $dom = new DOMDocument;
    $dom->loadXML($body);

    foreach ($dom->getElementsByTagName('script') as $node) {
        $node->parentNode->removeChild($node);
    }

    return $dom->saveXML() ?: '';
}