If drupal gets an empty session id cookie it does not regenerate the session id.
For example this cookie: "SESSd3e7335532b02436b9b16c4908fabcde=;". drupal creates a row with empty SID instead and it actually works for everyone who sends empty sid (session data will be shared between them).
In php session management this doesnt happen and a new sid will be generated and send to browser.
In drupal because it handles the session system itself on sess_open nothing get checked which i think it should be.
We fixed this by adding this lines in sess_open function but its not very good way to do this:
function sess_open($save_path, $session_name) {
$sess_id = session_id();
if (empty($sess_id)) {
$new_id = md5(uniqid(rand(999,999999999)));
session_id($new_id);
setcookie(session_name(), $new_id, 0, "/", $_SERVER['SERVER_NAME']);
}
return TRUE;
}

I hope you get to manage sessions more advanced in future.

Thank you

Comments

amin_pro’s picture

amin_pro CreditAttribution: amin_pro commented
Issue tags: -Sessions.inc +Session.inc
Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented
Version: x.y.z » 7.x-dev

I guess this is requested for D7

kbahey’s picture

kbahey CreditAttribution: kbahey commented

We had this happen to a Drupal 6.x site.

It just starting happening: users would complain that they are logged in as some other user.
Asking the user to track what is in the SESS* cookie, we found it to be empty in their browser.

This causes the query in sess_read() to return a user who has no sid.

$user = db_fetch_object(db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = '%s'", $

And therefore everyone gets logged in as that user.

The fix was to change the query as follows:

diff --git a/includes/session.inc b/includes/session.inc
index df4719c..6668ba8 100644
--- a/includes/session.inc
+++ b/includes/session.inc
@@ -48,7 +48,10 @@ function sess_read($key) {
   }
 
   // Otherwise, if the session is still active, we have a record of the client's session in the database.
-  $user = db_fetch_object(db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = '%s'", $
+  $user = db_fetch_object(db_query("SELECT u.*, s.*
+    FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid
+    WHERE s.sid = '%s'
+    AND sid <> ''", $key));
 
   // We found the client's session record and they are an authenticated,
   // active user.

Now for Drupal 7, the function _drupal_session_read() is a bit more complex because of considering secure cookies and plain cookies.

The above is defensive coding though. The real issue is on what conditions the sid ends up empty in the sessions table.

duozersk’s picture

duozersk
he/him
Primary language Russian
Location Moscow
CreditAttribution: duozersk commented

Khalid,

We are facing exactly the same behaviour on our site build on Pressflow 6 - users are complaining that they are getting logged in as other users.
And we also got records in sessions DB table with empty "sid" and session cookie SESS* is empty on the users' browsers. So they all (who has this empty value for the session cookie) are getting logged in as that user whose session has empty sid in the sessions DB table.

Did you manage to understand the reasoning behind the empty sid value there in the sessions DB table? Our check on the DB log queries shows that it gets inserted in the sess_write() function:

@db_query("INSERT INTO {sessions} (sid, uid, cache, hostname, session, timestamp) VALUES ('%s', %d, %d, '%s', '%s', %d)", $key, $user->uid, isset($user->cache) ? $user->cache : '', ip_address(), $value, time());

And we found exactly this DB query to set the empty sid value.

But then we need to understand why the sess_write() function receives the empty $key parameter that gets inserted as the sid.

Thank you in advance, hope to hear from you soon.
AndyB

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Status: Active » Postponed (maintainer needs more info)

The side effect of this (users taking over other users' sessions with empty session IDs) should have been fixed for both Drupal 6 and 7 as a result of https://www.drupal.org/SA-CORE-2014-006.

We don't know for sure how this happens in Drupal 6, but based on the fact that two people reported it happening for Drupal 6 in this issue it seems likely there was some avenue for that to occur, and the intention of the code in that security advisory was to prevent it from ever resulting in a vulnerability.

It still seems like there is some kind of bug (at least in Drupal 6) where an empty session ID can be unexpectedly written to the database... I'm leaving this issue open for that.

If there are any remaining problems involving the ability of a user to take over some other user's session, please report them privately to the Drupal security team rather than here.