SA-CORE-2010-001 - Blocked user session regeneration

I ran into a bug where a deleted session kept being recreated with the same sid. I had an authenticated user crawling my site with a bot causing some performance problems. Once discovered, I tried removing his session from the session database table in the hopes of forcing him to re-log (and break the crawler/bot). However, he was able to quickly re-create the same session with the exact same sid/uid pair.

With some digging, I was finally able to reproduce this bug. It looks like this was a combination of a lagging server and using a stock PHP session function. To reproduce, I created a page that had a sleep(10); in it. When using this test page to reproduce:

1. User starts loading the page
2. The user's session is manually removed from the database
3. User's page finishes execution, firing all shutdown functions
4. User's session is closed, recreating the sid/uid pair in the database (caused by the register_shutdown_function('session_write_close'); call in session.inc, I believe)

This basically means that if someone can slow your site down with enough traffic, you cannot drop their PHP session. Even after disabling the user, he was still able to crawl our site with his original session. Would this be considered a security issue?

Trying to fix this problem, I added a check for $user->status > 0 when reading a session (code below, found in includes/session.inc). By doing this, even if a user can recreate their session, they won't be treated as authenticated.

function sess_read($key) {
  global $user;

  // Write and Close handlers are called after destructing objects since PHP 5.0.5
  // Thus destructors can use sessions but session handler can't use objects.
  // So we are moving session closure before destructing objects.
  register_shutdown_function('session_write_close');

  // Handle the case of first time visitors and clients that don't store cookies (eg. web crawlers).
  if (!isset($_COOKIE[session_name()])) {
    $user = drupal_anonymous_user();
    return '';
  }

  // 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'", $key));

  // We found the client's session record and they are an authenticated user
  if ($user && $user->uid > 0 && $user->status > 0) {	 // <--- CHECK FOR STATUS HERE!!!
    // This is done to unserialize the data member of $user
    $user = drupal_unpack($user);

    // Add roles element to $user
    $user->roles = array();
    $user->roles[DRUPAL_AUTHENTICATED_RID] = 'authenticated user';
    $result = db_query("SELECT r.rid, r.name FROM {role} r INNER JOIN {users_roles} ur ON ur.rid = r.rid WHERE ur.uid = %d", $user->uid);
    while ($role = db_fetch_object($result)) {
      $user->roles[$role->rid] = $role->name;
    }
  }
  // We didn't find the client's record (session has expired), or they are an anonymous user.
  else {
    $session = isset($user->session) ? $user->session : '';
    $user = drupal_anonymous_user($session);
  }

  return $user->session;
}

Feedback welcome!