Fix security issue where it was possible to bypass the CSS jail in HTML messages using :root pseudo-class (#6897)

5 years ago · 554a20fe49
parent c0c42d1075
commit 554a20fe49
3 changed files with 71 additions and 1 deletions
--- a/1
+++ b/1
@ -19,6 +19,7 @@ RELEASE 1.3.10
 - 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 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)

 RELEASE 1.3.9
 -------------
--- a/program/lib/Roundcube/rcube_utils.php
+++ b/program/lib/Roundcube/rcube_utils.php
@ -429,7 +429,7 @@ class rcube_utils
            $source   = substr_replace($source, $repl, $pos+1, $length);
            $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(
@ -447,6 +447,39 @@ class rcube_utils
                $container_id,
            ),
            $source);
+*/
+        // remove html comments
+        $source = preg_replace('/(^\s*<\!--)|(-->\s*$)/m', '', $source);
+
+        // add #container to each tag selector
+        if ($container_id) {
+            // (?!##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 (stripos($replace, ':root') === 0) {
+                    $replace = substr($replace, 5);
+                }
+
+                $replace = "#$container_id " . $replace;
+
+                // Remove redundant spaces (for simpler testing)
+                $replace = preg_replace('/\s+/', ' ', $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);
--- a/tests/Framework/Utils.php
+++ b/tests/Framework/Utils.php
@ -239,6 +239,42 @@ class Framework_Utils extends PHPUnit_Framework_TestCase
        $this->assertContains("#rcmbody { background-image: url(http://example.com);", $mod, "Strict URIs in url() allowed with \$allow_remote=true");
    }

+    /**
+     * 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; }
+            }
+            :root * { color: red; }
+            :root > * { top: 0; }
+        ';
+        $mod = rcube_utils::mod_css_styles($css, 'rc', true);
+
+        $this->assertContains('#rc .one', $mod);
+        $this->assertContains('#rc .three.four', $mod);
+        $this->assertContains('#rc #id1', $mod);
+        $this->assertContains('#rc #id2.class:focus', $mod);
+        $this->assertContains('#rc .five:not(.test)', $mod);
+        $this->assertContains('#rc div .six', $mod);
+        $this->assertContains('#rc p > i ', $mod);
+        $this->assertContains('#rc div#some', $mod);
+        $this->assertContains('#rc li a.button', $mod);
+        $this->assertNotContains(':root', $mod);
+        $this->assertContains('#rc * ', $mod);
+        $this->assertContains('#rc > * ', $mod);
+    }
+
    function test_xss_entity_decode()
    {
        $mod = rcube_utils::xss_entity_decode("&lt;img/src=x onerror=alert(1)// </b>");