Bug 480224 - Squirrelmail session management broken by security backport
Summary: Squirrelmail session management broken by security backport
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: squirrelmail
Version: 4.7
Hardware: All
OS: Linux
high
urgent
Target Milestone: rc
: ---
Assignee: Michal Hlavinka
QA Contact: BaseOS QE
URL:
Whiteboard:
Depends On:
Blocks: CVE-2009-0030
TreeView+ depends on / blocked
 
Reported: 2009-01-15 19:08 UTC by Dan Astoorian
Modified: 2018-10-20 01:23 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-01-19 21:17:41 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Possible fix (1.33 KB, patch)
2009-01-16 15:24 UTC, Tomas Hoger
no flags Details | Diff
Updated patch (2.46 KB, patch)
2009-01-17 19:10 UTC, Tomas Hoger
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2009:0057 0 normal SHIPPED_LIVE Important: squirrelmail security update 2009-01-19 21:17:40 UTC

Description Dan Astoorian 2009-01-15 19:08:11 UTC
Description of problem:
After upgrading to squirrelmail-1.4.8-5.el4_7.2, user sessions may inadvertently be created with a session ID of "deleted" instead of a random identifier; if this occurs with multiple users, they will each have the same session ID (of "deleted"), and information (e.g., folder lists) can leak from one user's instance to another's.  Other interference can also occur (e.g., a user ending their squirrelmail session will also log out other users with the same session ID).

Version-Release number of selected component (if applicable):
squirrelmail-1.4.8-5.el4_7.2

How reproducible:
Always

Steps to Reproduce:
1. Exit and restart your web browser, if needed.
2. Visit: http://localhost/webmail/src/webmail.php.  You will correctly get the "You must be logged in to access this page" error.
3. Click through to the login page and log in.
  
Actual results:
If you examine the SQMSESSID cookie of the session, you will see that it has a value of "deleted".  Also, if you look in the PHP session directory (/var/lib/php/session/), you will see a session file called "sess_deleted" containing information related to the session.

Expected results:
SQMSESSID should contain a randomly-generated session identifier, and /var/lib/php/session/sess_deleted should never be created.

Additional info:
This did not occur in the previous release (squirrelmail-1.4.8-4.0.1.el4).

The following patch to functions/global.php appears to treat the symptom, but is not a complete fix:

--- /usr/share/squirrelmail/functions/global.php        2008-12-01 07:23:39.000000000 -0500
+++ global.php  2009-01-15 14:00:28.000000000 -0500
@@ -427,6 +427,10 @@
     @session_start();
     // could be: sq_call_function_suppress_errors('session_start');
     $session_id = session_id();
+    if ($session_id == 'deleted') {
+        session_regenerate_id();
+        $session_id = session_id();
+    }
 
     // session_starts sets the sessionid cookie but without the httponly var
     // setting the cookie again sets the httponly cookie attribute

Comment 1 Dan Astoorian 2009-01-15 22:58:53 UTC
Comparing functions/global.php in the Red Hat RPM to the main Squirrelmail distribution turns up a plausible cause for the problem: in sqsession_destroy(), Red Hat has:

    if (isset($_COOKIE[session_name()])) sqsetcookie(session_name(), '', 0, $base_uri);
    if (isset($_COOKIE['username'])) sqsetcookie('username', '', 0, $base_uri);
    if (isset($_COOKIE['key'])) sqsetcookie('key', '', 0, $base_uri);

whereas Squirrelmail 1.4.17 has:

    if (isset($_COOKIE[session_name()])) sqsetcookie(session_name(), $_COOKIE[session_name()], 1, $base_uri);
    if (isset($_COOKIE['key'])) sqsetcookie('key', 'SQMTRASH', 1, $base_uri);

Note that the third parameter to sqsetcookie() is set to 1 in SM1.4.17 and 0 in RH.

Inspection of the sqsetcookie() function shows that the expires header is set in the cookie only if empty($iExpire) is false.  Thus, an expiry parameter of 0 means that the cookie is not expired, compared with 1 meaning that the cookie is set to expire at 00:00:01 on January 1, 1970.

This could explain the observed symptom, although I have not tested the fix (replacing the 0s with 1s).

Comment 2 Michal Hlavinka 2009-01-16 11:38:18 UTC
I'm able to reproduce this.

If you've tested replacing 0s with 1s, please let me know the result.

Comment 3 Tomas Hoger 2009-01-16 15:24:52 UTC
Created attachment 329212 [details]
Possible fix

Only setting the expiration in sqsession_destroy() does not seem to be sufficient, as one of the SQMSESSID cookies remain set, so the 'deleted' will be session identifier for a user until the browser is restarted.

We should also uses non-empty second argument to sqsetcookie when unsetting session id cookie, as that's where 'deleted' gets introduced.

Patch also contains a change proposed in comment #0, as that seems to be the only way to resolve the problem that got created without forcing all users which already have bad cookies to restart their browsers.

Comment 5 Dan Astoorian 2009-01-16 15:57:17 UTC
Actually, 'deleted' comes from PHP, not from the default argument to sqsetcookie.  See the comments in
http://sourceforge.net/tracker/index.php?func=detail&aid=1829098&group_id=311&atid=100311
for more information.  Try changing the formal argument in sqsetcookie from "deleted" to something else and reproduce the problem: you'll find that the contents of the cookie is still the word "deleted."

Changing 1s to 0s seems to fix the symptom for me.  

However: if you log into Squirrelmail over https, then log out, and then follow the "Click here to sign back in" link, a cookie for a new session is sent without the "secure" attribute (i.e., Firefox reports "Send For: Any type of connection"), so the backport of the secure-cookie fix appears to be incomplete?

Comment 6 Tomas Hoger 2009-01-16 16:30:44 UTC
(In reply to comment #5)
> Actually, 'deleted' comes from PHP, not from the default argument to
> sqsetcookie.

Well, yes and no, as squirrelmail use both PHP's session handling (which seems to use setcookie internally) and own sqsetcookie, which does not use PHP's setcookie.  PHP's setcookie seems to set cookie value to 'deleted' when expiration time in the past is set.

> Try changing the formal argument in sqsetcookie from "deleted" to something
> else and reproduce the problem: you'll find that the contents of the cookie
> is still the word "deleted."

There are two 'deleted's in sqsetcookie.  sqsectookie called from sqsession_destroy() gets empty string as an argument, so the formal argument default value is not used, but deleted is set later:
  if (!$sValue) $sValue = 'deleted';

Comment 7 Tomas Hoger 2009-01-16 16:42:50 UTC
(In reply to comment #5)
> However: if you log into Squirrelmail over https, then log out, and then follow
> the "Click here to sign back in" link, a cookie for a new session is sent
> without the "secure" attribute (i.e., Firefox reports "Send For: Any type of
> connection"), so the backport of the secure-cookie fix appears to be
> incomplete?

Possibly any other step is involved?  I don't seem to be able to reproduce...

Comment 8 Dan Astoorian 2009-01-16 17:20:30 UTC
I'm seeing the insecure cookie issue on our production server, but haven't succeeded in reproducing it from a fresh installation (will report back if I'm able to do so).  On a fresh installation, I'm seeing a different issue: I'm seeing SQMSESSID cookies with different paths for some reason (/webmail/ vs. /webmail/src/).  In comment #3 you say "*one of* the SQMSESSID cookies remain set": are you also seeing multiple SQMSESSID cookies, and if so, do they have different paths?  There should normally only ever be one SQMSESSID cookie in the browser for a particular squirrelmail installation at any given time (and sqm_baseuri() in strings.php appears to try to make it the one without the src/ suffix).

Re comment #6, I missed the second occurrence of "deleted" in sqsetcookie--you're right about the value being set to "deleted" there.  However, the value of a cookie with an expiration in the distant past should be irrelevant--the browser is just going to delete the cookie anyway.

Why is this bug's Priority: set to "low"?

Comment 9 Rob Henderson 2009-01-16 19:50:32 UTC
We installed squirrelmail-1.4.8-5.el4_7.2 on our production server at 15:43 on 1/13.  I just got a report from a user that an email he sent 4 minutes later at 15:47 was sent with the From: address of some other user on our system.  YIKES!!!!  Interestingly, he said right after he hit 'Send' on that message he got an error about incorrect username or password and it kicked him to the login page.

I assume this is another manifestation of this session bug so this is obviously a critical issues for us (ie. not "low" priority :-)

I'm off to back out to squirrelmail-1.4.8-4.0.1.el4 unless anyone has a better recommendation for a quick workaround.

Comment 10 Dan Astoorian 2009-01-16 20:46:20 UTC
Re: comments #6-#7: I found the reason for the discrepancy: our production server has the Squirrelmail compatibility plugin installed, with its patch to functions/strings.php applied to include plugins/compatibility/functions.php .

It appears that this has the effect that a consistent path is used for all cookies ("/webmail/"), but it also seems to result in the apparent insecure cookie being set (by PHP, not the squirrelmail code) when the proposed fix is applied.  This "insecure cookie" is visible in Firefox from Edit -> Preferences -> Privacy -> Show Cookies..., but it disappears from the list if the Cookies window is closed and re-opened, so this may just be a browser artifact.

If the fact that Red Hat's squirrelmail issues cookies with different paths set ("/webmail/" and "/webmail/src/") is not an operational problem (and it probably isn't), then Tomas's proposed fix looks like the right move.

The symptom described in comment #9 is indeed one of the manifestations of this bug, and the author may wish to consider applying Tomas's patch.  He may also wish to note that the other user's identity may have been written into his preferences under /var/lib/squirrelmail/prefs/ as a result of the session confusion, so even after applying the fix he may need to reestablish his identity in Squirrelmail under Options -> Personal Information, or alternatively by deleting the .pref file.

Comment 11 Rob Henderson 2009-01-16 21:16:16 UTC
Thanks, Dan.  I did, in fact, find 3 user pref files that had been stepped on with contents from another user.

Comment 12 Jonathan Abbey 2009-01-16 21:21:49 UTC
We were bitten by this one as well, complete with mail 'from' incorrect users, scrambled permanent prefs files, strange 'References:' headers, and more.  We had tracked it down to the 'deleted' session conflation before coming across this bug report.

We applied Tomas' patch and it seems to have ameliorated the situation for us.

I also definitely wouldn't consider this a 'low' priority bug.  One of our directors got involved with this. ;-/

Comment 13 Peter C. Lai 2009-01-17 03:50:07 UTC
I applied this patch; it seems to be holding up. The main effect that I saw is that in the php session-store, sess_deleted, if it shows up, will be zeroed out which I guess must be the proper behavior. (Whereas when the bug was live, sess_deleted actually contained live session information).

<soapbox>Clobbering sessions is certainly one giant security issue though, one way worse than the vulnerabilities that the update were supposed to patch... While we cannot be 100% sure, we really hope that nobody was able to actually read anybody else's mail during the time the bug had been reported to us (our users pretty much saw all 3 effects: incorrect preference application [can't verify clobbering], incorrect mailbox enumeration, wrong sender being used, and continuous prompting to re-login).</soapbox>

Comment 14 Dan Astoorian 2009-01-17 15:25:19 UTC
Re: comment #13:

The patch (Tomas's attachment, not the workaround one I pasted into the original description) makes two adjustments.  One of them (described in comments #1 to #3) fixes the actual bug and should prevent sess_deleted from being created in the first place.  The second (described in the original description in this bug) makes Squirrelmail abandon sessions named "deleted" when a browser tries to establish or continue them, and has the effect of zeroing out the sess_deleted file: this mitigates the bug for anyone who's already logged in with a "deleted" session ID prior to the fix being applied.  Once all bad sessions have been blown away, the zero-size sess_deleted file should stop showing up.

My experience was that after applying the workaround fix I posted to the original comment, sess_deleted was zeroed out; after applying the rest of the fix, sess_deleted eventually stopped getting updated entirely and finally disappeared.

As far as I can tell, this bug should pose no risk of users gaining arbitrary access to each others' received mail in a typical configuration with login authentication to an IMAP server.  Squirrelmail, as far as I know, must send the user's password to the IMAP server for each transaction, and that information is not contained in the session: the password is split between the session and the browser, so you need both the "key" cookie stored in the browser at login time and the one-time pad stored in the session on the server in order to reconstruct the password and log into the IMAP server successfully.  (Failure to log into the IMAP server when the key and the pad are mismatched is almost certainly the cause of the symptom where Squirrelmail keeps prompting the user to re-login.)

What is at risk of disclosure between users or corruption is anything Squirrelmail itself stores for the session or the user, which may include user preferences (including identites), the folder list, address books, signatures, and perhaps uploaded attachments for outbound mail messages.

Comment 15 Tomas Hoger 2009-01-17 19:05:29 UTC
(In reply to comment #8)
> I'm seeing the insecure cookie issue on our production server, but haven't
> succeeded in reproducing it from a fresh installation (will report back if I'm
> able to do so).

You may want to try checking cookie path as well.

> On a fresh installation, I'm seeing a different issue: I'm seeing SQMSESSID
> cookies with different paths for some reason (/webmail/ vs. /webmail/src/).
> In comment #3 you say "*one of* the SQMSESSID cookies remain set": are you
> also seeing multiple SQMSESSID cookies, and if so, do they have different
> paths?

Yes, this seems to be a side affect of the setcookie -> sqsetcookie change introduced upstream.

> There should normally only ever be one SQMSESSID cookie in the browser for
> a particular squirrelmail installation at any given time (and sqm_baseuri()
> in strings.php appears to try to make it the one without the src/ suffix).

That's correct, but the $base_uri is not initialized for all pages, only seems to be set for the pages that are displayed in the top-most frames.  For others, cookie is set without path, resulting in browser using path of the script, i.e. with src/.

That problem is not specific to Red Hat backport, same problem exists in the latest upstream 1.4.17.  The extraneous src/ SQMSESSID cookie make this problem even bit worse.

> Re comment #6, I missed the second occurrence of "deleted" in
> sqsetcookie--you're right about the value being set to "deleted" there. 
> However, the value of a cookie with an expiration in the distant past should
> be irrelevant--the browser is just going to delete the cookie anyway.

The code never tried to clean up /webmail/src/ cookie, only /webmail/ one.

> Why is this bug's Priority: set to "low"?

Bugzilla default value.


Even though the same SquirrelMail packages are shipped in Red Hat Enterprise Linux 3, 4 and 5, RHEL5 does not seem to get into this bad state by user only logging out and re-logging in without closing web browser.  PHP4 and PHP5 versions seem to use different ways to deal with multiple values of the same  cookie specified in the requests (PHP4 uses last value, PHP5 first).  So on PHP5, it's more unlikely to get into this state, but bit more difficult to recover form it, due to an extraneous /src cookie (without restarting browser).

As the SQMSESSID is "session cookie", browser restart should be sufficient to get rid of "bad" cookie values (unless the browser is configured to use non-standard cookie handling).

Comment 16 Tomas Hoger 2009-01-17 19:10:14 UTC
Created attachment 329288 [details]
Updated patch

This change should prevent creation of /src/ cookie, as well as try to delete/expire existing one (if any exist on the user's side) during the session destroy.

Comment 18 Jon Jensen 2009-01-19 19:08:33 UTC
I can confirm this bug in production on RHEL 3. It was found because of cross-user data disclosure. In one case, one user saw another's folders in the left pane. In another case, a user's outgoing email had the headers of another user!

Comment 20 errata-xmlrpc 2009-01-19 21:17:41 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2009-0057.html

Comment 21 Tru Huynh 2009-01-20 17:34:05 UTC
just 2 cents: the src.rpm are not yet available on ftp://updates.redhat.com as of Tue Jan 20 17:33:44 UTC 2009

Comment 24 Joe Pruett 2009-04-17 17:45:53 UTC
there is still a problem related to this (or so i think).  i have an extra plugin for squirrelmail loaded (the gpg module) and i am getting multiple SQMSESSID cookies for different paths and this causes grief under php 5 systems because of the new order that cookies are used.  squirrelmail sees an old cookie and kicks you out.  restarting the browser or clearing the cookies is a temporary fix.

i've modified the plugin to use sqsetcookie but that hasn't helped.  i'm going to dig in more now, but i wanted to let people know that something still isn't happy.


Note You need to log in before you can comment on or make changes to this bug.