From: pdontthink <pdontthink@7612ce4b-ef26-0410-bec9-ea0150e637f0>
Date: Mon, 11 May 2009 21:49:23 +0000 (+0000)
Subject: Fixed improper sanitizing of PHP_SELF and the lack of sanitizing of QUERY_STRING... 
X-Git-Url: https://vcs.fsf.org/?p=squirrelmail.git;a=commitdiff_plain;h=960b7ec2d43bd8d3b15be292b7558a18182056a7

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
---

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="><script>window.open("http://example.com")</script>
+ * 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);