diff --git a/CHANGELOG b/CHANGELOG index 8eabe0f3f..f4a017651 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ CHANGELOG Roundcube Webmail =========================== +- Security: Fix cross-site scripting (XSS) via HTML or Plain text messages with malicious content [CVE-2020-35730] + RELEASE 1.3.15 -------------- - Security: Fix cross-site scripting (XSS) via HTML messages with malicious svg content [CVE-2020-16145] diff --git a/program/lib/Roundcube/rcube_string_replacer.php b/program/lib/Roundcube/rcube_string_replacer.php index 284d58547..d4ec20f23 100644 --- a/program/lib/Roundcube/rcube_string_replacer.php +++ b/program/lib/Roundcube/rcube_string_replacer.php @@ -24,7 +24,7 @@ */ class rcube_string_replacer { - public static $pattern = '/##str_replacement_(\d+)##/'; + public $pattern; public $mailto_pattern; public $link_pattern; public $linkref_index; @@ -39,6 +39,10 @@ class rcube_string_replacer function __construct($options = array()) { + // Create hard-to-guess replacement string + $uniq_ident = sprintf('%010d%010d', mt_rand(), mt_rand()); + $this->pattern = '/##' . $uniq_ident . '##(\d+)##/'; + // Simplified domain expression for UTF8 characters handling // Support unicode/punycode in top-level domain part $utf_domain = '[^?&@"\'\\/()<>\s\r\t\n]+\\.?([^\\x00-\\x2f\\x3b-\\x40\\x5b-\\x60\\x7b-\\x7f]{2,}|xn--[a-zA-Z0-9]{2,})'; @@ -49,7 +53,7 @@ class rcube_string_replacer $link_prefix = "([\w]+:\/\/|{$this->noword}[Ww][Ww][Ww]\.|^[Ww][Ww][Ww]\.)"; $this->options = $options; - $this->linkref_index = '/\[([^\]#]+)\](:?\s*##str_replacement_(\d+)##)/'; + $this->linkref_index = '/\[([^\]#]+)\](:?\s*' . substr($this->pattern, 1, -1) . ')/'; $this->linkref_pattern = '/\[([^\]#]+)\]/'; $this->link_pattern = "/$link_prefix($utf_domain([$url1]*[$url2]+)*)/"; $this->mailto_pattern = "/(" @@ -78,7 +82,7 @@ class rcube_string_replacer */ public function get_replacement($i) { - return '##str_replacement_' . $i . '##'; + return str_replace('(\d+)', $i, substr($this->pattern, 1, -1)); } /** @@ -121,7 +125,7 @@ class rcube_string_replacer public function linkref_addindex($matches) { $key = $matches[1]; - $this->linkrefs[$key] = $this->urls[$matches[3]]; + $this->linkrefs[$key] = isset($this->urls[$matches[3]]) ? $this->urls[$matches[3]] : null; return $this->get_replacement($this->add('['.$key.']')) . $matches[2]; } @@ -166,7 +170,7 @@ class rcube_string_replacer */ public function replace_callback($matches) { - return $this->values[$matches[1]]; + return isset($this->values[$matches[1]]) ? $this->values[$matches[1]] : null; } /** @@ -193,7 +197,7 @@ class rcube_string_replacer */ public function resolve($str) { - return preg_replace_callback(self::$pattern, array($this, 'replace_callback'), $str); + return preg_replace_callback($this->pattern, array($this, 'replace_callback'), $str); } /** diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php index fc7a579d8..c75a64144 100644 --- a/program/lib/Roundcube/rcube_utils.php +++ b/program/lib/Roundcube/rcube_utils.php @@ -451,12 +451,12 @@ class rcube_utils // remove html comments $source = preg_replace('/(^\s*<\!--)|(-->\s*$)/m', '', $source); - // add #container to each tag selector + // add #container to each tag selector and prefix to id/class identifiers 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'; + // Exclude rcube_string_replacer pattern matches, this is needed + // for cases like @media { body { position: fixed; } } (#5811) + $excl = '(?!' . substr($replacements->pattern, 1, -1) . ')'; + $regexp = '/(^\s*|,\s*|\}\s*|\{\s*)(' . $excl . ':?[a-z0-9\._#\*\[][a-z0-9\._:\(\)#=~ \[\]"\|\>\+\$\^-]*)/im'; $callback = function($matches) use ($container_id, $prefix) { $replace = $matches[2]; diff --git a/tests/Framework/Text2Html.php b/tests/Framework/Text2Html.php index c85e44a4c..db2dbac31 100644 --- a/tests/Framework/Text2Html.php +++ b/tests/Framework/Text2Html.php @@ -120,4 +120,21 @@ class Framework_Text2Html extends PHPUnit_Framework_TestCase $this->assertEquals($output, $html); } + + /** + * Test XSS issue + */ + function test_text2html_xss() + { + $input = "\n[]:##str_replacement_0##\n"; + $t2h = new rcube_text2html($input); + + $html = $t2h->get_html(); + + $expected = "

\n" + . "[<script>evil</script>]:##str_replacement_0##
\n" + . "
"; + + $this->assertEquals($expected, $html); + } }