Remove ability for HTML emails to use CSS positioning to overlay SquirrelMail content...

git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@13668 7612ce4b-ef26-0410-bec9-ea0150e637f0

diff --git a/doc/ChangeLog b/doc/ChangeLog
index a920eb79303c4a01553d7604bd437b528ee73d23..726a5680537da018de34ab7a9083a1e4b5af3f50 100644 (file)
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -293,6 +293,8 @@ Version 1.5.2 - SVN
   - Completed a massive update to contrib/flat2sql.pl.
   - Display visual indication of forwarded messages.
   - Added Khmer translation (Thanks to Khoem Sokhem).
+  - Remove ability for HTML emails to use CSS positioning to overlay
+    SquirrelMail content (Thanks to Luc Beurton). (#2723196) [CVE-2009-1581]
 
 Version 1.5.1 (branched on 2006-02-12)
 --------------------------------------

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(
                     "",