diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php index 06f4314b7..bcb9258b4 100644 --- a/program/lib/Roundcube/rcube_utils.php +++ b/program/lib/Roundcube/rcube_utils.php @@ -380,10 +380,11 @@ class rcube_utils /** * Replace all css definitions with #container [def] - * and remove css-inlined scripting + * and remove css-inlined scripting, make position style safe * * @param string CSS source code * @param string Container ID to use as prefix + * @param bool Allow remote content * * @return string Modified CSS source */ @@ -411,6 +412,9 @@ class rcube_utils $length = $pos2 - $pos - 1; $styles = substr($source, $pos+1, $length); + // Convert position:fixed to position:absolute (#5264) + $styles = preg_replace('/position:[\s\r\n]*fixed/i', 'position: absolute', $styles); + // check every line of a style block... if ($allow_remote) { $a_styles = preg_split('/;[\r\n]*/', $styles, -1, PREG_SPLIT_NO_EMPTY); diff --git a/program/lib/Roundcube/rcube_washtml.php b/program/lib/Roundcube/rcube_washtml.php index d03f04af4..21e28bb59 100644 --- a/program/lib/Roundcube/rcube_washtml.php +++ b/program/lib/Roundcube/rcube_washtml.php @@ -235,6 +235,11 @@ class rcube_washtml } } else if (!preg_match('/^(behavior|expression)/i', $val)) { + // Set position:fixed to position:absolute for security (#5264) + if (!strcasecmp($cssid, 'position') && !strcasecmp($val, 'fixed')) { + $val = 'absolute'; + } + // whitelist ? $value .= ' ' . $val; @@ -727,10 +732,9 @@ class rcube_washtml */ protected function explode_style($style) { - $style = trim($style); + $pos = 0; // first remove comments - $pos = 0; while (($pos = strpos($style, '/*', $pos)) !== false) { $end = strpos($style, '*/', $pos+2); @@ -742,6 +746,7 @@ class rcube_washtml } } + $style = trim($style); $strlen = strlen($style); $result = array(); diff --git a/tests/Framework/Utils.php b/tests/Framework/Utils.php index 08e641440..82c0017bc 100644 --- a/tests/Framework/Utils.php +++ b/tests/Framework/Utils.php @@ -204,6 +204,16 @@ class Framework_Utils extends PHPUnit_Framework_TestCase $mod = rcube_utils::mod_css_styles("background:\\0075\\0072\\006c( javascript:alert('xss') )", 'rcmbody'); $this->assertEquals("/* evil! */", $mod, "Don't allow encoding quirks (2)"); + + // position: fixed (#5264) + $mod = rcube_utils::mod_css_styles(".test { position: fixed; }", 'rcmbody'); + $this->assertEquals("#rcmbody .test { position: absolute; }", $mod, "Replace position:fixed with position:absolute (0)"); + + $mod = rcube_utils::mod_css_styles(".test { position:\nfixed; }", 'rcmbody'); + $this->assertEquals("#rcmbody .test { position: absolute; }", $mod, "Replace position:fixed with position:absolute (1)"); + + $mod = rcube_utils::mod_css_styles(".test { position:/**/fixed; }", 'rcmbody'); + $this->assertEquals("#rcmbody .test { position: absolute; }", $mod, "Replace position:fixed with position:absolute (2)"); } /** diff --git a/tests/Framework/Washtml.php b/tests/Framework/Washtml.php index ef4b2e9a0..865a9b3d0 100644 --- a/tests/Framework/Washtml.php +++ b/tests/Framework/Washtml.php @@ -274,4 +274,18 @@ class Framework_Washtml extends PHPUnit_Framework_TestCase $this->assertSame($washed, $exp, "SVG content"); } + + /** + * Test position:fixed cleanup - (#5264) + */ + function test_style_wash_position_fixed() + { + $html = ""; + $exp = ""; + + $washer = new rcube_washtml; + $washed = $washer->wash($html); + + $this->assertTrue(strpos($washed, $exp) !== false, "Position:fixed (#5264)"); + } }