Fix css conflicts in user interface and e-mail content (#5891)

... by adding prefix to element/class identifiers Also cleaned up some code and removed global variable use.
roundcube · Oct 12, 2017 · 3196d65 · 3196d65
1 parent 403d845
commit 3196d65
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 44 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -41,6 +41,7 @@ CHANGELOG Roundcube Webmail
 - Localized timezone selector (#4983)
 - Use 7bit encoding for ISO-2022-* charsets in sent mail (#5640)
 - Handle inline images also inside multipart/mixed messages (#5905)
+- Fix css conflicts in user interface and e-mail content (#5891)
 - Fix duplicated signature when using Back button in Chrome (#5809)
 - Fix touch event issue on messages list in IE/Edge (#5781)
 - Fix so links over images are not removed in plain text signatures converted from HTML (#4473)

diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php
@@ -373,10 +373,11 @@ public static function html_identifier($str, $encode=false)
      * @param string CSS source code
      * @param string Container ID to use as prefix
      * @param bool   Allow remote content
+     * @param string Prefix to be added to id/class identifier
      *
      * @return string Modified CSS source
      */
-    public static function mod_css_styles($source, $container_id, $allow_remote = false)
+    public static function mod_css_styles($source, $container_id, $allow_remote = false, $prefix = '')
     {
         $last_pos     = 0;
         $replacements = new rcube_string_replacer;
@@ -432,23 +433,37 @@ public static function mod_css_styles($source, $container_id, $allow_remote = fa
             $last_pos = $pos2 - ($length - strlen($repl));
         }
 
-        // remove html comments and add #container to each tag selector.
-        // also replace body definition because we also stripped off the <body> tag
-        $source = preg_replace(
-            array(
-                '/(^\s*<\!--)|(-->\s*$)/m',
-                // (?!##str) below is to not match with ##str_replacement_0##
-                // from rcube_string_replacer used above, this is needed for
-                // cases like @media { body { position: fixed; } } (#5811)
-                '/(^\s*|,\s*|\}\s*|\{\s*)((?!##str)[a-z0-9\._#\*][a-z0-9\.\-_]*)/im',
-                '/'.preg_quote($container_id, '/').'\s+body/i',
-            ),
-            array(
-                '',
-                "\\1#$container_id \\2",
-                $container_id,
-            ),
-            $source);
+        // remove html comments
+        $source = preg_replace('/(^\s*<\!--)|(-->\s*$)/m', '', $source);
+
+        // add #container to each tag selector and prefix to id/class identifiers
+        if ($container_id || $prefix) {
+            // (?!##str) below is to not match with ##str_replacement_0##
+            // from rcube_string_replacer used above, this is needed for
+            // cases like @media { body { position: fixed; } } (#5811)
+            $regexp   = '/(^\s*|,\s*|\}\s*|\{\s*)((?!##str)[a-z0-9\._#\*\[][a-z0-9\._:\(\)#=~ \[\]"\|\>\+\$\^-]*)/im';
+            $callback = function($matches) use ($container_id, $prefix) {
+                $replace = $matches[2];
+
+                if ($prefix) {
+                    $replace = str_replace(array('.', '#'), array(".$prefix", "#$prefix"), $replace);
+                }
+
+                if ($container_id) {
+                    $replace = "#$container_id " . $replace;
+                }
+
+                return str_replace($matches[2], $replace, $matches[0]);
+            };
+
+            $source = preg_replace_callback($regexp, $callback, $source);
+        }
+
+        // replace body definition because we also stripped off the <body> tag
+        if ($container_id) {
+            $regexp = '/#' . preg_quote($container_id, '/') . '\s+body/i';
+            $source = preg_replace($regexp, "#$container_id", $source);
+        }
 
         // put block contents back in
         $source = $replacements->resolve($source);

diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php
@@ -132,9 +132,9 @@ class rcube_washtml
         'bordercolordark', 'face', 'marginwidth', 'marginheight', 'axis', 'border',
         'abbr', 'char', 'charoff', 'clear', 'compact', 'coords', 'vspace', 'hspace',
         'cellborder', 'size', 'lang', 'dir', 'usemap', 'shape', 'media',
-        'background', 'src', 'poster', 'href',
+        'background', 'src', 'poster', 'href', 'headers',
         // attributes of form elements
-        'type', 'rows', 'cols', 'disabled', 'readonly', 'checked', 'multiple', 'value',
+        'type', 'rows', 'cols', 'disabled', 'readonly', 'checked', 'multiple', 'value', 'for',
         // SVG
         'accent-height', 'accumulate', 'additive', 'alignment-baseline', 'alphabetic',
         'ascent', 'attributename', 'attributetype', 'azimuth', 'basefrequency', 'baseprofile',
@@ -203,6 +203,9 @@ class rcube_washtml
     /* Allowed HTML attributes */
     private $_html_attribs = array();
 
+    /* A prefix to be added to id/class/for attribute values */
+    private $_css_prefix;
+
     /* Max nesting level */
     private $max_nesting_level;
 
@@ -218,6 +221,7 @@ public function __construct($p = array())
         $this->_html_attribs    = array_flip((array)$p['html_attribs']) + array_flip(self::$html_attribs);
         $this->_ignore_elements = array_flip((array)$p['ignore_elements']) + array_flip(self::$ignore_elements);
         $this->_void_elements   = array_flip((array)$p['void_elements']) + array_flip(self::$void_elements);
+        $this->_css_prefix      = is_string($p['css_prefix']) && strlen($p['css_prefix']) ? $p['css_prefix'] : null;
 
         unset($p['html_elements'], $p['html_attribs'], $p['ignore_elements'], $p['void_elements']);
 
@@ -338,6 +342,9 @@ private function wash_attribs($node)
                         $out = $value;
                     }
                 }
+                else if ($this->_css_prefix !== null && in_array($key, array('id', 'class', 'for'))) {
+                    $out = preg_replace('/(\S+)/', $this->_css_prefix . '\1', $value);
+                }
                 else if ($key) {
                    $out = $value;
                 }

diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc
@@ -875,6 +875,8 @@ function rcmail_wash_html($html, $p, $cid_replaces = array())
         'charset'       => RCUBE_CHARSET,
         'cid_map'       => $cid_replaces,
         'html_elements' => array('body'),
+        'css_prefix'    => $p['css_prefix'],
+        'container_id'  => $p['container_id'],
     );
 
     if (!$p['inline_html']) {
@@ -886,10 +888,12 @@ function rcmail_wash_html($html, $p, $cid_replaces = array())
     }
 
     // overwrite washer options with options from plugins
-    if (isset($p['html_elements']))
+    if (isset($p['html_elements'])) {
         $wash_opts['html_elements'] = $p['html_elements'];
-    if (isset($p['html_attribs']))
+    }
+    if (isset($p['html_attribs'])) {
         $wash_opts['html_attribs'] = $p['html_attribs'];
+    }
 
     // initialize HTML washer
     $washer = new rcube_washtml($wash_opts);
@@ -908,8 +912,9 @@ function rcmail_wash_html($html, $p, $cid_replaces = array())
         $washer->add_callback('a', 'rcmail_washtml_link_callback');
         $washer->add_callback('area', 'rcmail_washtml_link_callback');
 
-        if ($p['safe'])
+        if ($p['safe']) {
             $washer->add_callback('link', 'rcmail_washtml_link_callback');
+        }
     }
 
     // Remove non-UTF8 characters (#1487813)
@@ -1314,19 +1319,25 @@ function rcmail_message_body($attrib)
                 $container_id     = $container_class . (++$part_no);
                 $container_attrib = array('class' => $container_class, 'id' => $container_id);
 
-                // Assign container ID to a global variable for use in rcmail_washtml_link_callback()
-                $GLOBALS['rcmail_html_container_id'] = $container_id;
+                $body_args = array(
+                    'safe'         => $safe_mode,
+                    'plain'        => !$RCMAIL->config->get('prefer_html'),
+                    'css_prefix'   => 'v' . $part_no,
+                    'body_class'   => 'rcmBody',
+                    'container_id'     => $container_id,
+                    'container_attrib' => $container_attrib,
+                );
 
                 // Parse the part content for display
-                $body = rcmail_print_body($body, $part, array('safe' => $safe_mode, 'plain' => !$RCMAIL->config->get('prefer_html')));
+                $body = rcmail_print_body($body, $part, $body_args);
 
                 // check if the message body is PGP encrypted
                 if (strpos($body, '-----BEGIN PGP MESSAGE-----') !== false) {
                     $OUTPUT->set_env('is_pgp_content', '#' . $container_id);
                 }
 
                 if ($part->ctype_secondary == 'html') {
-                    $body = rcmail_html4inline($body, $container_id, 'rcmBody', $container_attrib, $safe_mode);
+                    $body = rcmail_html4inline($body, $body_args);
                 }
 
                 $out .= html::div($container_attrib, $plugin['prefix'] . $body);
@@ -1469,22 +1480,22 @@ function rcmail_part_image_type($part)
 /**
  * Modify a HTML message that it can be displayed inside a HTML page
  */
-function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=array(), $allow_remote=false)
+function rcmail_html4inline($body, $args)
 {
-    $last_style_pos = 0;
-    $cont_id        = $container_id . ($body_class ? ' div.'.$body_class : '');
+    $last_pos = 0;
+    $cont_id  = $args['container_id'] . ($args['body_class'] ? ' div.' . $args['body_class'] : '');
 
     // find STYLE tags
-    while (($pos = stripos($body, '<style', $last_style_pos)) && ($pos2 = stripos($body, '</style>', $pos))) {
+    while (($pos = stripos($body, '<style', $last_pos)) && ($pos2 = stripos($body, '</style>', $pos))) {
         $pos = strpos($body, '>', $pos) + 1;
         $len = $pos2 - $pos;
 
         // replace all css definitions with #container [def]
         $styles = substr($body, $pos, $len);
-        $styles = rcube_utils::mod_css_styles($styles, $cont_id, $allow_remote);
+        $styles = rcube_utils::mod_css_styles($styles, $cont_id, $args['safe'], $args['css_prefix']);
 
-        $body = substr_replace($body, $styles, $pos, $len);
-        $last_style_pos = $pos2 + strlen($styles) - $len;
+        $body     = substr_replace($body, $styles, $pos, $len);
+        $last_pos = $pos2 + strlen($styles) - $len;
     }
 
     $body = preg_replace(array(
@@ -1511,13 +1522,13 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=a
             '<!--\\1-->',
             '&lt;?',
             '?&gt;',
-            '<div class="' . $body_class . '"\\1>',
+            '<div class="' . $args['body_class'] . '"\\1>',
             '</div>',
         ),
         $body);
 
     // Handle body attributes that doesn't play nicely with div elements
-    $regexp = '/<div class="' . preg_quote($body_class, '/') . '"([^>]*)/';
+    $regexp = '/<div class="' . preg_quote($args['body_class'], '/') . '"([^>]*)/';
     if (preg_match($regexp, $body, $m)) {
         $style = array();
         $attrs = $m[0];
@@ -1563,7 +1574,7 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=a
     // make sure there's 'rcmBody' div, we need it for proper css modification
     // its name is hardcoded in rcmail_message_body() also
     else {
-        $body = '<div class="' . $body_class . '">' . $body . '</div>';
+        $body = '<div class="' . $args['body_class'] . '">' . $body . '</div>';
     }
 
     return $body;
@@ -1586,8 +1597,15 @@ function rcmail_washtml_link_callback($tag, $attribs, $content, $washtml)
     if ($tag == 'link' && preg_match('/^https?:\/\//i', $attrib['href'])) {
         $tempurl = 'tmp-' . md5($attrib['href']) . '.css';
         $_SESSION['modcssurls'][$tempurl] = $attrib['href'];
-        $attrib['href'] = $RCMAIL->url(array('task' => 'utils', 'action' => 'modcss', 'u' => $tempurl, 'c' => $GLOBALS['rcmail_html_container_id']));
+        $attrib['href'] = $RCMAIL->url(array(
+                'task'   => 'utils',
+                'action' => 'modcss',
+                'u'      => $tempurl,
+                'c'      => $washtml->get_config('container_id'),
+                'p'      => $washtml->get_config('css_prefix'),
+        ));
         $end = ' />';
+        $content = null;
     }
     else if (preg_match('/^mailto:(.+)/i', $attrib['href'], $mailto)) {
         list($mailto, $url) = explode('?', html_entity_decode($mailto[1], ENT_QUOTES, 'UTF-8'), 2);

diff --git a/program/steps/utils/modcss.inc b/program/steps/utils/modcss.inc
@@ -72,10 +72,12 @@ else {
 }
 
 $ctype_regexp = '~Content-Type:\s+text/(css|plain)~i';
+$container_id = preg_replace('/[^a-z0-9]/i', '', $_GET['_c']);
+$css_prefix   = preg_replace('/[^a-z0-9]/i', '', $_GET['_p']);
 
 if ($source !== false && preg_match($ctype_regexp, $headers)) {
     header('Content-Type: text/css');
-    echo rcube_utils::mod_css_styles($source, preg_replace('/[^a-z0-9]/i', '', $_GET['_c']));
+    echo rcube_utils::mod_css_styles($source, $container, false, $css_prefix);
     exit;
 }
 

diff --git a/tests/Framework/Utils.php b/tests/Framework/Utils.php
@@ -223,6 +223,37 @@ function test_mod_css_styles_xss()
         $this->assertContains("#rcmbody { background-image: url(data:image/png;base64,123);", $mod, "Data URIs in url() allowed [2]");
     }
 
+    /**
+     * rcube_utils::mod_css_styles()'s prefix argument handling
+     */
+    function test_mod_css_styles_prefix()
+    {
+        $css = '
+            .one { font-size: 10pt; }
+            .three.four { font-weight: bold; }
+            #id1 { color: red; }
+            #id2.class:focus { color: white; }
+            .five:not(.test), { background: transparent; }
+            div .six { position: absolute; }
+            p > i { font-size: 120%; }
+            div#some { color: yellow; }
+            @media screen and (max-width: 699px) and (min-width: 520px) {
+                li a.button { padding-left: 30px; }
+            }
+        ';
+        $mod = rcube_utils::mod_css_styles($css, 'rc', true, 'test');
+
+        $this->assertContains('#rc .testone', $mod);
+        $this->assertContains('#rc .testthree.testfour', $mod);
+        $this->assertContains('#rc #testid1', $mod);
+        $this->assertContains('#rc #testid2.testclass:focus', $mod);
+        $this->assertContains('#rc .testfive:not(.testtest)', $mod);
+        $this->assertContains('#rc div .testsix', $mod);
+        $this->assertContains('#rc p > i ', $mod);
+        $this->assertContains('#rc div#testsome', $mod);
+        $this->assertContains('#rc li a.testbutton', $mod);
+    }
+
     function test_xss_entity_decode()
     {
         $mod = rcube_utils::xss_entity_decode("&lt;img/src=x onerror=alert(1)// </b>");

diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php
@@ -369,4 +369,19 @@ function test_textarea_content_escaping()
         $this->assertNotContains('onerror=alert(1)>', $washed);
         $this->assertContains('&lt;p style=&quot;x:', $washed);
     }
+
+    /**
+     * Test css_prefix feature
+     */
+    function test_css_prefix()
+    {
+        $washer = new rcube_washtml(array('css_prefix' => 'test'));
+
+        $html   = '<p id="my-id"><label for="my-other-id" class="my-class1 my-class2">test</label></p>';
+        $washed = $washer->wash($html);
+
+        $this->assertContains('id="testmy-id"', $washed);
+        $this->assertContains('for="testmy-other-id"', $washed);
+        $this->assertContains('class="testmy-class1 testmy-class2"', $washed);
+    }
 }
diff --git a/tests/MailFunc.php b/tests/MailFunc.php
@@ -42,7 +42,7 @@ function test_html()
         $part->replaces = array('ex1.jpg' => 'part_1.2.jpg', 'ex2.jpg' => 'part_1.2.jpg');
 
         // render HTML in normal mode
-        $html = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => false)), 'foo');
+        $html = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => false)), array('container_id' => 'foo'));
 
         $this->assertRegExp('/src="'.$part->replaces['ex1.jpg'].'"/', $html, "Replace reference to inline image");
         $this->assertRegExp('#background="program/resources/blocked.gif"#', $html, "Replace external background image");
@@ -56,7 +56,7 @@ function test_html()
         $this->assertTrue($GLOBALS['REMOTE_OBJECTS'], "Remote object detected");
 
         // render HTML in safe mode
-        $html2 = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => true)), 'foo');
+        $html2 = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => true)), array('container_id' => 'foo'));
 
         $this->assertRegExp('/<style [^>]+>/', $html2, "Allow styles in safe mode");
         $this->assertRegExp('#src="http://evilsite.net/mailings/ex3.jpg"#', $html2, "Allow external images in HTML (safe mode)");
@@ -76,7 +76,7 @@ function test_html_xss()
         $this->assertNotRegExp('/src="skins/', $washed, "Remove local references");
         $this->assertNotRegExp('/\son[a-z]+/', $washed, "Remove on* attributes");
 
-        $html = rcmail_html4inline($washed, 'foo');
+        $html = rcmail_html4inline($washed, array('container_id' => 'foo'));
         $this->assertNotRegExp('/onclick="return rcmail.command(\'compose\',\'[email protected]\',this)"/', $html, "Clean mailto links");
         $this->assertNotRegExp('/alert/', $html, "Remove alerts");
     }
@@ -88,7 +88,8 @@ function test_html_xss()
     function test_html_xss2()
     {
         $part = $this->get_html_part('src/BID-26800.txt');
-        $washed = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => true)), 'dabody', '', $attr, true);
+        $washed = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => true)),
+             array('container_id' => 'dabody', 'safe' => true));
 
         $this->assertNotRegExp('/alert|expression|javascript|xss/', $washed, "Remove evil style blocks");
         $this->assertNotRegExp('/font-style:italic/', $washed, "Allow valid styles");
@@ -145,7 +146,7 @@ function test_mailto()
         $part = $this->get_html_part('src/mailto.txt');
 
         // render HTML in normal mode
-        $html = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => false)), 'foo');
+        $html = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => false)), array('container_id' => 'foo'));
 
         $mailto = '<a href="mailto:[email protected]"'
             .' onclick="return rcmail.command(\'compose\',\'[email protected]?subject=this is the subject&amp;body=this is the body\',this)" rel="noreferrer">e-mail</a>';