diff --git a/CHANGELOG b/CHANGELOG index e04f3c5dc..435921bfe 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -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 ------------- diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php index b519e8147..fc7a579d8 100644 --- 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 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 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/tests/Framework/Utils.php b/tests/Framework/Utils.php index beeddc618..274ae0770 100644 --- 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("<img/src=x onerror=alert(1)// ");