Anchor the regexp. Thanks Thijs Kinkhorst.

[squirrelmail.git] / functions / mime.php

diff --git a/functions/mime.php b/functions/mime.php
index 72e41843b72f1d56340218ac3490a37d8115d39f..349b3bd212d2b45e606e4b07309a8221128e31dd 100644 (file)
--- a/functions/mime.php
+++ b/functions/mime.php
@@ -2143,6 +2143,12 @@ function sq_fixstyle($body, $pos, $message, $id, $mailbox){
     /**
      * Fix stupid css declarations which lead to vulnerabilities
      * in IE.
+     *
+     * Also remove "position" attribute, as it can easily be set
+     * to "fixed" or "absolute" with "left" and "top" attributes
+     * of zero, taking over the whole content frame.  It can also
+     * be set to relative and move itself anywhere it wants to,
+     * displaying content in areas it shouldn't be allowed to touch.
      */
     $match   = Array('/\/\*.*\*\//',
                     '/expression/i',
@@ -2150,8 +2156,9 @@ function sq_fixstyle($body, $pos, $message, $id, $mailbox){
                     '/binding/i',
                     '/include-source/i',
                     '/javascript/i',
-                    '/script/i');
-    $replace = Array('','idiocy', 'idiocy', 'idiocy', 'idiocy', 'idiocy', 'idiocy');
+                    '/script/i',
+                    '/position/i');
+    $replace = Array('','idiocy', 'idiocy', 'idiocy', 'idiocy', 'idiocy', 'idiocy', '');
     $contentNew = preg_replace($match, $replace, $contentTemp);
     if ($contentNew !== $contentTemp) {
         // insecure css declarations are used. From now on we don't care
@@ -2556,12 +2563,28 @@ function magicHTML($body, $id, $message, $mailbox = 'INBOX', $take_mailto_links
                     "/binding/i",
                     "/behaviou*r/i",
                     "/include-source/i",
-                    "/position\s*:\s*absolute/i",
+
+                    // position:relative can also be exploited
+                    // to put content outside of email body area
+                    // and position:fixed is similarly exploitable
+                    // as position:absolute, so we'll remove it
+                    // altogether....
+                    //
+                    // Does this screw up legitimate HTML messages?
+                    // If so, the only fix I see is to allow position
+                    // attributes (any values?  I think we still have
+                    // to block static and fixed) only if $use_iframe
+                    // is enabled (1.5.0+)
+                    //
+                    // was:   "/position\s*:\s*absolute/i",
+                    //
+                    "/position\s*:/i",
+
                     "/(\\\\)?u(\\\\)?r(\\\\)?l(\\\\)?/i",
                     "/url\s*\(\s*([\'\"])\s*\S+script\s*:.*([\'\"])\s*\)/si",
                     "/url\s*\(\s*([\'\"])\s*mocha\s*:.*([\'\"])\s*\)/si",
                     "/url\s*\(\s*([\'\"])\s*about\s*:.*([\'\"])\s*\)/si",
-                    "/(.*)\s*:\s*url\s*\(\s*([\'\"]*)\s*\S+script\s*:.*([\'\"]*)\s*\)/si"
+                    "/(.*)\s*:\s*url\s*\(\s*([\'\"]*)\s*\S+script\s*:.*([\'\"]*)\s*\)/si",
                     ),
                 Array(
                     "",