Wash position:fixed style in HTML mail for better security (#5264)

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);

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();
 

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)");
     }
 
     /**

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 = "<img style='position:fixed' /><img style=\"position:/**/ fixed; top:10px\" />";
+        $exp  = "<img style=\"position: absolute\" /><img style=\"position: absolute; top: 10px\" />";
+
+        $washer = new rcube_washtml;
+        $washed = $washer->wash($html);
+
+        $this->assertTrue(strpos($washed, $exp) !== false, "Position:fixed (#5264)");
+    }
 }