From 425e31dc275ec0fb1c3231e4ffa51589dd0d5de4 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <machniak@kolabsys.com>
Date: Sun, 29 May 2016 17:09:41 +0200
Subject: [PATCH] Wash position:fixed style in HTML mail for better security
 (#5264)

---
 program/lib/Roundcube/rcube_utils.php   |  6 +++++-
 program/lib/Roundcube/rcube_washtml.php |  9 +++++++--
 tests/Framework/Utils.php               | 10 ++++++++++
 tests/Framework/Washtml.php             | 14 ++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php
index 580f65578..6780ed5cb 100644
--- a/program/lib/Roundcube/rcube_utils.php
+++ b/program/lib/Roundcube/rcube_utils.php
@@ -419,10 +419,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
      */
@@ -450,6 +451,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 6535e3b4a..1d5d2b4c0 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 572a19c00..9a6257eda 100644
--- a/tests/Framework/Utils.php
+++ b/tests/Framework/Utils.php
@@ -199,6 +199,16 @@ class Framework_Utils extends PHPUnit_Framework_TestCase
 
         $mod = rcube_utils::mod_css_styles("background:\\0075\\0072\\006c( javascript:alert(&#039;xss&#039;) )", '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 2e681791c..50454cd4e 100644
--- a/tests/Framework/Washtml.php
+++ b/tests/Framework/Washtml.php
@@ -269,4 +269,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)");
+    }
 }