Fix XSS problem with unsanitized style tags in messages [CVE-2011-2023]
[squirrelmail.git] / functions / mime.php
index ca7f836c0affe9722c52f5b89ecc92b61eea8892..1e8fc880c1b82237c8d316e0851b3e6cca0a489b 100644 (file)
@@ -6,7 +6,7 @@
  * This contains the functions necessary to detect and decode MIME
  * messages.
  *
- * @copyright © 1999-2007 The SquirrelMail Project Team
+ * @copyright 1999-2011 The SquirrelMail Project Team
  * @license http://opensource.org/licenses/gpl-license.php GNU Public License
  * @version $Id$
  * @package squirrelmail
@@ -92,6 +92,9 @@ function mime_structure ($bodystructure, $flags=array()) {
                     if (strtolower($flag) == '\\flagged') {
                         $msg->is_flagged = true;
                     }
+                    else if (strtolower($flag) == '$forwarded') {
+                        $msg->is_forwarded = true;
+                    }
                     break;
                 case 'M':
                     if (strtolower($flag) == '$mdnsent') {
@@ -137,7 +140,7 @@ function mime_fetch_body($imap_stream, $id, $ent_id=1, $fetch_size=0) {
     } while($topline && ($topline[0] == '*') && !preg_match('/\* [0-9]+ FETCH.*/i', $topline)) ;
 
     $wholemessage = implode('', $data);
-    if (ereg('\\{([^\\}]*)\\}', $topline, $regs)) {
+    if (preg_match('/\{([^\}]*)\}/', $topline, $regs)) {
         $ret = substr($wholemessage, 0, $regs[1]);
         /* There is some information in the content info header that could be important
          * in order to parse html messages. Let's get them here.
@@ -145,7 +148,7 @@ function mime_fetch_body($imap_stream, $id, $ent_id=1, $fetch_size=0) {
 //        if ($ret{0} == '<') {
 //            $data = sqimap_run_command ($imap_stream, "FETCH $id BODY[$ent_id.MIME]", true, $response, $message, TRUE);
 //        }
-    } else if (ereg('"([^"]*)"', $topline, $regs)) {
+    } else if (preg_match('/"([^"]*)"/', $topline, $regs)) {
         $ret = $regs[1];
     } else if ((stristr($topline, 'nil') !== false) && (empty($wholemessage))) {
         $ret = $wholemessage;
@@ -572,9 +575,9 @@ function buildAttachmentArray($message, $exclude_id, $mailbox, $id) {
         }
 
         /* This executes the attachment hook with a specific MIME-type.
-         * If that doesn't have results, it tries if there's a rule
-         * for a more generic type. Finally, a hook for ALL attachment
-         * types is run as well.
+         * It also allows plugins to run if there's a rule for a more
+         * generic type. Finally, a hook for ALL attachment types is
+         * run as well.
          */
         // First remember the default link.
         $defaultlink_orig = $defaultlink;
@@ -584,19 +587,23 @@ function buildAttachmentArray($message, $exclude_id, $mailbox, $id) {
            argument, and arguments are passed by reference, so instead of
            returning any changes, changes should simply be made to the original
            arguments themselves. */
-        $temp = array(&$links, &$startMessage, &$id, &$urlMailbox, &$ent, 
+        $temp = array(&$links, &$startMessage, &$id, &$urlMailbox, &$ent,
                     &$defaultlink, &$display_filename, &$where, &$what);
         do_hook("attachment $type0/$type1", $temp);
-        if(count($links) <= 1 && $defaultlink == $defaultlink_orig) {
-            /* The API for this hook has changed as of 1.5.2 so that all plugin
-               arguments are passed in an array instead of each their own plugin
-               argument, and arguments are passed by reference, so instead of
-               returning any changes, changes should simply be made to the original
-               arguments themselves. */
-            $temp = array(&$links, &$startMessage, &$id, &$urlMailbox, &$ent, 
-                          &$defaultlink, &$display_filename, &$where, &$what);
-            do_hook("attachment $type0/*", $temp);
+        /* The API for this hook has changed as of 1.5.2 so that all plugin
+           arguments are passed in an array instead of each their own plugin
+           argument, and arguments are passed by reference, so instead of
+           returning any changes, changes should simply be made to the original
+           arguments themselves. */
+        $temp = array(&$links, &$startMessage, &$id, &$urlMailbox, &$ent,
+                      &$defaultlink, &$display_filename, &$where, &$what);
+        // Do not let a generic plugin change the default link if a more
+        // specialized one already did it...
+        if ($defaultlink != $defaultlink_orig) {
+            $dummy = '';
+            $temp[5] = &$dummy;
         }
+        do_hook("attachment $type0/*", $temp);
         /* The API for this hook has changed as of 1.5.2 so that all plugin
            arguments are passed in an array instead of each their own plugin
            argument, and arguments are passed by reference, so instead of
@@ -706,7 +713,7 @@ function sqimap_base64_decode(&$string) {
  *                           all be converted to LF; if "CRLF",
  *                           line endings will all be converted
  *                           to CRLF.  If given as an empty value,
- *                           the global $default_force_crlf will
+ *                           the global $force_crlf_default will
  *                           be consulted (it can be specified in
  *                           config/config_local.php).  Otherwise,
  *                           any other value will cause the string
@@ -955,6 +962,7 @@ function encodeHeader ($string) {
     for($i = 0; $i < $j; ++$i) {
         switch($string{$i})
         {
+            case '"':
             case '=':
             case '<':
             case '>':
@@ -2140,6 +2148,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',
@@ -2147,8 +2161,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
@@ -2335,6 +2350,15 @@ function sq_sanitize($body,
             list($free_content, $curpos) =
                 sq_fixstyle($body, $gt+1, $message, $id, $mailbox);
             if ($free_content != FALSE){
+                $attary = sq_fixatts($tagname,
+                                     $attary,
+                                     $rm_attnames,
+                                     $bad_attvals,
+                                     $add_attr_to_tag,
+                                     $message,
+                                     $id,
+                                     $mailbox
+                                     );
                 $trusted .= sq_tagprint($tagname, $attary, $tagtype);
                 $trusted .= $free_content;
                 $trusted .= sq_tagprint($tagname, false, 2);
@@ -2553,12 +2577,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(
                     "",
@@ -2708,7 +2748,7 @@ function SendDownloadHeaders($type0, $type1, $filename, $force, $filesize=0) {
         $filename =
             call_user_func($languages[$squirrelmail_language]['XTRA_CODE'] . '_downloadfilename', $filename, $HTTP_USER_AGENT);
     } else {
-        $filename = ereg_replace('[\\/:\*\?"<>\|;]', '_', str_replace('&nbsp;', ' ', $filename));
+        $filename = preg_replace('/[\\\\\/:*?"<>|;]/', '_', str_replace('&nbsp;', ' ', $filename));
     }
 
     // A Pox on Microsoft and it's Internet Explorer!