From 39352565ef2e2f1d438b23a5c291ec9e655ae8a0 Mon Sep 17 00:00:00 2001 From: pdontthink Date: Mon, 11 May 2009 21:19:52 +0000 Subject: [PATCH] Remove ability for HTML emails to use CSS positioning to overlay SquirrelMail content. Thanks to Luc Beurton. (#2723196/CVE-2009-1581) git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@13668 7612ce4b-ef26-0410-bec9-ea0150e637f0 --- doc/ChangeLog | 2 ++ functions/mime.php | 31 +++++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/doc/ChangeLog b/doc/ChangeLog index a920eb79..726a5680 100644 --- 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 72e41843..349b3bd2 100644 --- 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( "", -- 2.25.1