From: kink Date: Wed, 9 May 2007 14:01:13 +0000 (+0000) Subject: Security: fixes for the HTML filter to counter further XSS exploits: X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=567dc5244e08bf50998e3ac590c64674b72de53d;hp=f258865ca7ffb4f19de9f6c656ba64748d1e6072;p=squirrelmail.git Security: fixes for the HTML filter to counter further XSS exploits: HTML attachments containing 'data:' URLs, Internet Explorer-specifc charset conversion exploits, and request forgery through included images. Thanks to Mikhail Markin, Tomas Kuliavas and Michael Jordon for reporting these issues. [CVE-2007-1262] git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@12371 7612ce4b-ef26-0410-bec9-ea0150e637f0 --- diff --git a/ChangeLog b/ChangeLog index cc243d03..27eb5a5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -193,6 +193,11 @@ Version 1.5.2 - SVN domains. If plugins use this function, it fixes #1434043. - Add dynamic textarea sizing slider control to compose screen (default_advanced skin) + - Security: fixes for the HTML filter to counter further XSS exploits: + HTML attachments containing 'data:' URLs, Internet Explorer-specifc + charset conversion exploits, and request forgery through included + images. Thanks to Mikhail Markin, Tomas Kuliavas and Michael Jordon + for reporting these issues. [CVE-2007-1262] Version 1.5.1 (branched on 2006-02-12) -------------------------------------- diff --git a/functions/mime.php b/functions/mime.php index 5ff5e79b..da8ee6a1 100644 --- a/functions/mime.php +++ b/functions/mime.php @@ -426,13 +426,16 @@ function formatBody($imap_stream, $message, $color, $wrap_at, $ent_num, $id, $ma $body = $oTemplate->fetch('read_html_iframe.tpl'); } else { // old way of html rendering - $body = magicHTML($body, $id, $message, $mailbox); /** * convert character set. charset_decode does not remove html special chars * applied by magicHTML functions and does not sanitize them second time if * fourth argument is true. */ - $body = charset_decode($body_message->header->getParameter('charset'),$body,false,true); + $charset = $body_message->header->getParameter('charset'); + if (!empty($charset)) { + $body = charset_decode($charset,$body,false,true); + } + $body = magicHTML($body, $id, $message, $mailbox); } } else { translateText($body, $wrap_at, @@ -1196,8 +1199,8 @@ function sq_fixIE_idiocy(&$attvalue) { array('ʟ', 'ʟ' ,/* L UNICODE IPA Extension */ 'ʀ', 'ʀ' ,/* R UNICODE IPA Extension */ 'ɴ', 'ɴ' ,/* N UNICODE IPA Extension */ - 'Ｅ', 'Ｅ' ,/* Unicode FULLWIDTH LATIN CAPITAL LETTER E */ - 'ｅ', 'ｅ' ,/* Unicode FULLWIDTH LATIN SMALL LETTER E */ + 'Ｅ', 'Ｅ' ,/* Unicode FULLWIDTH LATIN CAPITAL LETTER E */ + 'ｅ', 'ｅ' ,/* Unicode FULLWIDTH LATIN SMALL LETTER E */ 'Ｘ', 'Ｘ',/* Unicode FULLWIDTH LATIN CAPITAL LETTER X */ 'ｘ', 'ｘ',/* Unicode FULLWIDTH LATIN SMALL LETTER X */ 'Ｐ', 'Ｐ',/* Unicode FULLWIDTH LATIN CAPITAL LETTER P */ @@ -1217,26 +1220,34 @@ function sq_fixIE_idiocy(&$attvalue) { 'Ｕ', 'Ｕ',/* Unicode FULLWIDTH LATIN CAPITAL LETTER U */ 'ｕ', 'ｕ',/* Unicode FULLWIDTH LATIN SMALL LETTER U */ 'ⁿ', 'ⁿ' ,/* Unicode SUPERSCRIPT LATIN SMALL LETTER N */ - '艤', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER E */ // in unicode this is some Chinese char range - '芅', /* Shift JIS FULLWIDTH LATIN SMALL LETTER E */ - '艷', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER X */ - '芘', /* Shift JIS FULLWIDTH LATIN SMALL LETTER X */ - '良', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER P */ - '芐', /* Shift JIS FULLWIDTH LATIN SMALL LETTER P */ - '艱', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER R */ - '芒', /* Shift JIS FULLWIDTH LATIN SMALL LETTER R */ - '色', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER S */ - '芓', /* Shift JIS FULLWIDTH LATIN SMALL LETTER S */ - '艨', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER I */ - '芉', /* Shift JIS FULLWIDTH LATIN SMALL LETTER I */ - '艮', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER O */ - '芏', /* Shift JIS FULLWIDTH LATIN SMALL LETTER O */ - '艭', /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER N */ - '芎'), /* Shift JIS FULLWIDTH LATIN SMALL LETTER N */ + "\xEF\xBC\xA5", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER E */ // in unicode this is some Chinese char range + "\xEF\xBD\x85", /* Shift JIS FULLWIDTH LATIN SMALL LETTER E */ + "\xEF\xBC\xB8", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER X */ + "\xEF\xBD\x98", /* Shift JIS FULLWIDTH LATIN SMALL LETTER X */ + "\xEF\xBC\xB0", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER P */ + "\xEF\xBD\x90", /* Shift JIS FULLWIDTH LATIN SMALL LETTER P */ + "\xEF\xBC\xB2", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER R */ + "\xEF\xBD\x92", /* Shift JIS FULLWIDTH LATIN SMALL LETTER R */ + "\xEF\xBC\xB3", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER S */ + "\xEF\xBD\x93", /* Shift JIS FULLWIDTH LATIN SMALL LETTER S */ + "\xEF\xBC\xA9", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER I */ + "\xEF\xBD\x89", /* Shift JIS FULLWIDTH LATIN SMALL LETTER I */ + "\xEF\xBC\xAF", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER O */ + "\xEF\xBD\x8F", /* Shift JIS FULLWIDTH LATIN SMALL LETTER O */ + "\xEF\xBC\xAE", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER N */ + "\xEF\xBD\x8E", /* Shift JIS FULLWIDTH LATIN SMALL LETTER N */ + "\xEF\xBC\xAC", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER L */ + "\xEF\xBD\x8C", /* Shift JIS FULLWIDTH LATIN SMALL LETTER L */ + "\xEF\xBC\xB5", /* Shift JIS FULLWIDTH LATIN CAPITAL LETTER U */ + "\xEF\xBD\x95", /* Shift JIS FULLWIDTH LATIN SMALL LETTER U */ + "\xE2\x81\xBF", /* Shift JIS FULLWIDTH SUPERSCRIPT N */ + "\xCA\x9F", /* L UNICODE IPA Extension */ + "\xCA\x80", /* R UNICODE IPA Extension */ + "\xC9\xB4"), /* N UNICODE IPA Extension */ array('l', 'l', 'r','r','n','n', - 'E','E','e','e','X','X','x','x','P','P','p','p','S','S','s','s','I','I', - 'i','i','O','O','o','o','N','N','n','n','L','L','l','l','U','U','u','u','n', - 'E','e','X','x','P','p','S','s','I','i','O','o','N','n')); + 'E','E','e','e','X','X','x','x','P','P','p','p','R','R','r','r','S','S','s','s','I','I', + 'i','i','O','O','o','o','N','N','n','n','L','L','l','l','U','U','u','u','n','n', + 'E','e','X','x','P','p','R','r','S','s','I','i','O','o','N','n','L','l','U','u','n','l','r','n')); $attvalue = str_replace($aDangerousCharsReplacementTable[0],$aDangerousCharsReplacementTable[1],$attvalue); // Escapes are useful for special characters like "{}[]()'&. In other cases they are @@ -1751,38 +1762,34 @@ function sq_fixatts($tagname, preg_replace($valmatch, $valrepl, $attvalue); if ($newvalue != $attvalue){ $attary{$attname} = $newvalue; + $attvalue = $newvalue; } } } } } - - /** - * Replace empty src tags with the blank image. src is only used - * for frames, images, and image inputs. Doing a replace should - * not affect them working as should be, however it will stop - * IE from being kicked off when src for img tags are not set - */ - if (($attname == 'src') && ($attvalue == '""')) { - $attary{$attname} = '"' . SM_PATH . 'images/blank.png"'; - } - - /** - * Turn cid: urls into http-friendly ones. - */ - if (preg_match("/^[\'\"]\s*cid:/si", $attvalue)){ - $attary{$attname} = sq_cid2http($message, $id, $attvalue, $mailbox); + if ($attname == 'style') { + if (preg_match('/[\0-\37\200-\377]+/',$attvalue)) { + // 8bit and control characters in style attribute values can be used for XSS, remove them + $attary{$attname} = '"disallowed character"'; + } + preg_match_all("/url\s*\((.+)\)/si",$attvalue,$aMatch); + if (count($aMatch)) { + foreach($aMatch[1] as $sMatch) { + // url value + $urlvalue = $sMatch; + sq_fix_url($attname, $urlvalue, $message, $id, $mailbox,"'"); + $attary{$attname} = str_replace($sMatch,$urlvalue,$attvalue); + } + } } - /** - * "Hack" fix for Outlook using propriatary outbind:// protocol in img tags. - * One day MS might actually make it match something useful, for now, falling - * back to using cid2http, so we can grab the blank.png. + * Use white list based filtering on attributes which can contain url's */ - if (preg_match("/^[\'\"]\s*outbind:\/\//si", $attvalue)) { - $attary{$attname} = sq_cid2http($message, $id, $attvalue, $mailbox); + else if ($attname == 'href' || $attname == 'src' || $attname == 'background') { + sq_fix_url($attname, $attvalue, $message, $id, $mailbox); + $attary{$attname} = $attvalue; } - } /** * See if we need to append any attributes to this tag. @@ -1795,6 +1802,98 @@ function sq_fixatts($tagname, return $attary; } +/** + * This function filters url's + * + * @param $attvalue String with attribute value to filter + * @param $message message object + * @param $id message id + * @param $mailbox mailbox + * @param $sQuote quoting characters around url's + */ +function sq_fix_url($attname, &$attvalue, $message, $id, $mailbox,$sQuote = '"') { + $attvalue = trim($attvalue); + if ($attvalue && ($attvalue[0] =='"'|| $attvalue[0] == "'")) { + // remove the double quotes + $sQuote = $attvalue[0]; + $attvalue = trim(substr($attvalue,1,-1)); + } + + if( !sqgetGlobalVar('view_unsafe_images', $view_unsafe_images, SQ_GET) ) { + $view_unsafe_images = false; + } + $secremoveimg = '../images/' . _("sec_remove_eng.png"); + + /** + * Replace empty src tags with the blank image. src is only used + * for frames, images, and image inputs. Doing a replace should + * not affect them working as should be, however it will stop + * IE from being kicked off when src for img tags are not set + */ + if ($attvalue == '') { + $attvalue = '"' . SM_PATH . 'images/blank.png"'; + } else { + // first, disallow 8 bit characters and control characters + if (preg_match('/[\0-\37\200-\377]+/',$attvalue)) { + switch ($attname) { + case 'href': + $attvalue = $sQuote . 'http://invalid-stuff-detected.example.com' . $sQuote; + break; + default: + $attvalue = $sQuote . SM_PATH . 'images/blank.png'. $sQuote; + break; + } + } else { + $aUrl = parse_url($attvalue); + if (isset($aUrl['scheme'])) { + switch(strtolower($aUrl['scheme'])) { + case 'http': + case 'https': + case 'ftp': + if ($attname != 'href') { + if ($view_unsafe_images == false) { + $attvalue = $sQuote . $secremoveimg . $sQuote; + } else { + if (isset($aUrl['path'])) { + // validate image extension. + $ext = strtolower(substr($aUrl['path'],strrpos($aUrl['path'],'.'))); + if (!in_array($ext,array('.jpeg','.jpg','xjpeg','.gif','.bmp','.jpe','.png','.xbm'))) { + $attvalue = $sQuote . SM_PATH . 'images/blank.png'. $sQuote; + } + } else { + $attvalue = $sQuote . SM_PATH . 'images/blank.png'. $sQuote; + } + } + } + break; + case 'outbind': + /** + * "Hack" fix for Outlook using propriatary outbind:// protocol in img tags. + * One day MS might actually make it match something useful, for now, falling + * back to using cid2http, so we can grab the blank.png. + */ + $attvalue = sq_cid2http($message, $id, $attvalue, $mailbox); + break; + case 'cid': + /** + * Turn cid: urls into http-friendly ones. + */ + $attvalue = sq_cid2http($message, $id, $attvalue, $mailbox); + break; + default: + $attvalue = $sQuote . SM_PATH . 'images/blank.png' . $sQuote; + break; + } + } else { + if (!(isset($aUrl['path']) && $aUrl['path'] == $secremoveimg)) { + // parse_url did not lead to satisfying result + $attvalue = $sQuote . SM_PATH . 'images/blank.png' . $sQuote; + } + } + } + } +} + /** * This function edits the style definition to make them friendly and * usable in SquirrelMail. @@ -1886,6 +1985,12 @@ function sq_fixstyle($body, $pos, $message, $id, $mailbox){ // $content = preg_replace("|url\s*\(\s*([\'\"])\s*\S+script\s*:.*?([\'\"])\s*\)|si", // "url(\\1$secremoveimg\\2)", $content); + // first check for 8bit sequences and disallowed control characters + if (preg_match('/[\16-\37\200-\377]+/',$content)) { + $content = ''; + return array($content, $newpos); + } + // IE Sucks hard. We have a special function for it. sq_fixIE_idiocy($content); @@ -1895,48 +2000,19 @@ function sq_fixstyle($body, $pos, $message, $id, $mailbox){ // translate ur\l and variations (IE parses that) // TODO check if the sq_fixIE_idiocy function already handles this. $content = preg_replace("/(\\\\)?u(\\\\)?r(\\\\)?l(\\\\)?/i", 'url', $content); - // NB I insert NUL characters to keep to avoid an infinite loop. They are removed after the loop. - while (preg_match("/url\s*\(\s*[\'\"]?([^:]+):(.*)?[\'\"]?\s*\)/si", $content, $matches)) { - $sProto = strtolower($matches[1]); - switch ($sProto) { - /** - * Fix url('https*://.*) declarations but only if $view_unsafe_images - * is false. - */ - case 'https': - case 'http': - if (!$view_unsafe_images){ - - $sExpr = "/url\s*\(\s*[\'\"]?\s*$sProto*:.*[\'\"]?\s*\)/si"; - $content = preg_replace($sExpr, "u\0r\0l(\\1$secremoveimg\\2)", $content); - - } else { - $content = preg_replace('/url/i',"u\0r\0l",$content); - } - break; - /** - * Fix urls that refer to cid: - */ - case 'cid': - $cidurl = 'cid:'. $matches[2]; - $httpurl = sq_cid2http($message, $id, $cidurl, $mailbox); - // escape parentheses that can modify the regular expression - $cidurl = str_replace(array('(',')'),array('\\(','\\)'),$cidurl); - $content = preg_replace("|url\s*\(\s*$cidurl\s*\)|si", - "u\0r\0l($httpurl)", $content); - break; - default: - /** - * replace url with protocol other then the white list - * http,https and cid by an empty string. - */ - $content = preg_replace("/url\s*\(\s*[\'\"]?([^:]+):(.*)?[\'\"]?\s*\)/si", - "", $content); - break; + preg_match_all("/url\s*\((.+)\)/si",$content,$aMatch); + if (count($aMatch)) { + $aValue = $aReplace = array(); + foreach($aMatch[1] as $sMatch) { + // url value + $urlvalue = $sMatch; + sq_fix_url('style',$urlvalue, $message, $id, $mailbox,"'"); + $aValue[] = $sMatch; + $aReplace[] = $urlvalue; } + $content = str_replace($aValue,$aReplace,$content); } - // remove NUL - $content = str_replace("\0", "", $content); + /** * Remove any backslashes, entities, and extraneous whitespace. */ @@ -2373,7 +2449,7 @@ function magicHTML($body, $id, $message, $mailbox = 'INBOX', $take_mailto_links "idiocy", "idiocy", "idiocy", - "", + "idiocy", "url", "url(\\1#\\1)", "url(\\1#\\1)", @@ -2419,7 +2495,7 @@ function magicHTML($body, $id, $message, $mailbox = 'INBOX', $take_mailto_links $id, $mailbox ); - if (preg_match("|$secremoveimg|i", $trusted)){ + if (strpos($trusted,$secremoveimg)){ $has_unsafe_images = true; } diff --git a/src/compose.php b/src/compose.php index 3f46038d..ae4eb311 100644 --- a/src/compose.php +++ b/src/compose.php @@ -45,31 +45,37 @@ sqgetGlobalVar('delayed_errors', $delayed_errors, SQ_SESSION); $oErrorHandler->setDelayedErrors(true); /** SESSION/POST/GET VARS */ -sqgetGlobalVar('session',$session); -sqgetGlobalVar('mailbox',$mailbox); -if(!sqgetGlobalVar('identity',$identity)) { +sqgetGlobalVar('send', $send, SQ_POST); +// Send can only be achieved by setting $_POST var. If Send = true then +// retrieve other form fields from $_POST +if (isset($send) && $send) { + $SQ_GLOBAL = SQ_POST; +} else { + $SQ_GLOBAL = SQ_FORM; +} +sqgetGlobalVar('session',$session, $SQ_GLOBAL); +sqgetGlobalVar('mailbox',$mailbox, $SQ_GLOBAL); +if(!sqgetGlobalVar('identity',$identity, $SQ_GLOBAL)) { $identity=0; } -sqgetGlobalVar('send_to',$send_to); -sqgetGlobalVar('send_to_cc',$send_to_cc); -sqgetGlobalVar('send_to_bcc',$send_to_bcc); -sqgetGlobalVar('subject',$subject); -sqgetGlobalVar('body',$body); -sqgetGlobalVar('mailprio',$mailprio); -sqgetGlobalVar('request_mdn',$request_mdn); -sqgetGlobalVar('request_dr',$request_dr); -sqgetGlobalVar('html_addr_search',$html_addr_search); -sqgetGlobalVar('mail_sent',$mail_sent); -sqgetGlobalVar('passed_id',$passed_id); -sqgetGlobalVar('passed_ent_id',$passed_ent_id); -sqgetGlobalVar('send',$send); - -sqgetGlobalVar('attach',$attach); - -sqgetGlobalVar('draft',$draft); -sqgetGlobalVar('draft_id',$draft_id); -sqgetGlobalVar('ent_num',$ent_num); -sqgetGlobalVar('saved_draft',$saved_draft); +sqgetGlobalVar('send_to',$send_to, $SQ_GLOBAL); +sqgetGlobalVar('send_to_cc',$send_to_cc, $SQ_GLOBAL); +sqgetGlobalVar('send_to_bcc',$send_to_bcc, $SQ_GLOBAL); +sqgetGlobalVar('subject',$subject, $SQ_GLOBAL); +sqgetGlobalVar('body',$body, $SQ_GLOBAL); +sqgetGlobalVar('mailprio',$mailprio, $SQ_GLOBAL); +sqgetGlobalVar('request_mdn',$request_mdn, $SQ_GLOBAL); +sqgetGlobalVar('request_dr',$request_dr, $SQ_GLOBAL); +sqgetGlobalVar('html_addr_search',$html_addr_search, $SQ_GLOBAL); +sqgetGlobalVar('mail_sent',$mail_sent, $SQ_GLOBAL); +sqgetGlobalVar('passed_id',$passed_id, $SQ_GLOBAL); +sqgetGlobalVar('passed_ent_id',$passed_ent_id, $SQ_GLOBAL); + +sqgetGlobalVar('attach',$attach, SQ_POST); +sqgetGlobalVar('draft',$draft, SQ_POST); +sqgetGlobalVar('draft_id',$draft_id, $SQ_GLOBAL); +sqgetGlobalVar('ent_num',$ent_num, $SQ_GLOBAL); +sqgetGlobalVar('saved_draft',$saved_draft, SQ_FORM); if ( sqgetGlobalVar('delete_draft',$delete_draft) ) { $delete_draft = (int)$delete_draft; diff --git a/src/view_text.php b/src/view_text.php index ce84feb4..e2ea3f02 100644 --- a/src/view_text.php +++ b/src/view_text.php @@ -65,11 +65,11 @@ if (isset($languages[$squirrelmail_language]['XTRA_CODE']) && if ($type1 == 'html' || (isset($override_type1) && $override_type1 == 'html')) { $ishtml = TRUE; - $body = MagicHTML( $body, $passed_id, $message, $mailbox); // html attachment with character set information if (! empty($charset)) { $body = charset_decode($charset,$body,false,true); } + $body = MagicHTML( $body, $passed_id, $message, $mailbox); } else { $ishtml = FALSE; translateText($body, $wrap_at, $charset);