From 960b7ec2d43bd8d3b15be292b7558a18182056a7 Mon Sep 17 00:00:00 2001 From: pdontthink Date: Mon, 11 May 2009 21:49:23 +0000 Subject: [PATCH] Fixed improper sanitizing of PHP_SELF and the lack of sanitizing of QUERY_STRING server environment variables. Thanks to Niels Teusink and Christian Balzer. (CVE-2009-1578) git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@13669 7612ce4b-ef26-0410-bec9-ea0150e637f0 --- doc/ChangeLog | 3 +++ include/init.php | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/doc/ChangeLog b/doc/ChangeLog index 726a5680..0eb45944 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -295,6 +295,9 @@ Version 1.5.2 - SVN - 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] + - Fixed improper sanitizing of PHP_SELF and the lack of sanitizing of + QUERY_STRING server environment variables. (Thanks to Niels Teusink + and Christian Balzer). [CVE-2009-1578] Version 1.5.1 (branched on 2006-02-12) -------------------------------------- diff --git a/include/init.php b/include/init.php index 9f17708e..047f1877 100644 --- a/include/init.php +++ b/include/init.php @@ -263,10 +263,21 @@ if (function_exists('get_magic_quotes_gpc') && @get_magic_quotes_gpc()) { } -/* strip any tags added to the url from PHP_SELF. -This fixes hand crafted url XXS expoits for any - page that uses PHP_SELF as the FORM action */ -$_SERVER['PHP_SELF'] = strip_tags($_SERVER['PHP_SELF']); +/** + * Strip any tags added to the url from PHP_SELF. + * This fixes hand crafted url XXS expoits for any + * page that uses PHP_SELF as the FORM action + * Update: strip_tags() won't catch something like + * src/right_main.php?sort=0&startMessage=1&mailbox=INBOX&xxx="> + * or + * contrib/decrypt_headers.php/%22%20onmouseover=%22alert(%27hello%20world%27)%22%3E + * because it doesn't bother with broken tags. + * htmlspecialchars() is the preferred method. + * QUERY_STRING also needs the same treatment since it is + * used in php_self(). + */ +$_SERVER['PHP_SELF'] = htmlspecialchars($_SERVER['PHP_SELF']); +$_SERVER['QUERY_STRING'] = htmlspecialchars($_SERVER['QUERY_STRING']); $PHP_SELF = php_self(); @@ -791,6 +802,7 @@ function checkForJavascript($reset = FALSE) { if ( !$reset && sqGetGlobalVar('javascript_on', $javascript_on, SQ_SESSION) ) return $javascript_on; + //FIXME: this isn't used anywhere else in this function; can we remove it? why is it here? $user_is_logged_in = FALSE; if ( $reset || !isset($javascript_setting) ) $javascript_setting = getPref($data_dir, $username, 'javascript_setting', SMPREF_JS_AUTODETECT); -- 2.25.1