diff options
| -rw-r--r-- | includes/session.inc | 9 | ||||
| -rw-r--r-- | modules/simpletest/tests/session.test | 24 | ||||
| -rw-r--r-- | modules/simpletest/tests/session_test.module | 26 | ||||
| -rw-r--r-- | modules/system/system.install | 5 |
4 files changed, 58 insertions, 6 deletions
diff --git a/includes/session.inc b/includes/session.inc index c23c23e1c..23af6bd64 100644 --- a/includes/session.inc +++ b/includes/session.inc @@ -88,10 +88,7 @@ function _drupal_session_read($sid) { // a HTTPS session or we are about to log in so we check the sessions table // for an anonymous session with the non-HTTPS-only cookie. if ($is_https) { - // Ensure that an empty secure session ID cannot be selected. - if ($sid) { - $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.ssid = :ssid", array(':ssid' => $sid))->fetchObject(); - } + $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.ssid = :ssid", array(':ssid' => $sid))->fetchObject(); if (!$user) { if (isset($_COOKIE[$insecure_session_name])) { $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid AND s.uid = 0", array( @@ -239,7 +236,9 @@ function drupal_session_initialize() { session_set_save_handler('_drupal_session_open', '_drupal_session_close', '_drupal_session_read', '_drupal_session_write', '_drupal_session_destroy', '_drupal_session_garbage_collection'); - if (isset($_COOKIE[session_name()]) || ($is_https && variable_get('https', FALSE) && isset($_COOKIE[substr(session_name(), 1)]))) { + // We use !empty() in the following check to ensure that blank session IDs + // are not valid. + if (!empty($_COOKIE[session_name()]) || ($is_https && variable_get('https', FALSE) && !empty($_COOKIE[substr(session_name(), 1)]))) { // If a session cookie exists, initialize the session. Otherwise the // session is only started on demand in drupal_session_commit(), making // anonymous users not use a session cookie unless something is stored in diff --git a/modules/simpletest/tests/session.test b/modules/simpletest/tests/session.test index f02cbef40..a42adcf36 100644 --- a/modules/simpletest/tests/session.test +++ b/modules/simpletest/tests/session.test @@ -223,6 +223,30 @@ class SessionTestCase extends DrupalWebTestCase { } /** + * Test that empty session IDs are not allowed. + */ + function testEmptySessionID() { + $user = $this->drupalCreateUser(array('access content')); + $this->drupalLogin($user); + $this->drupalGet('session-test/is-logged-in'); + $this->assertResponse(200, t('User is logged in.')); + + // Reset the sid in {sessions} to a blank string. This may exist in the + // wild in some cases, although we normally prevent it from happening. + db_query("UPDATE {sessions} SET sid = '' WHERE uid = :uid", array(':uid' => $user->uid)); + // Send a blank sid in the session cookie, and the session should no longer + // be valid. Closing the curl handler will stop the previous session ID + // from persisting. + $this->curlClose(); + $this->additionalCurlOptions[CURLOPT_COOKIE] = rawurlencode($this->session_name) . '=;'; + $this->drupalGet('session-test/id-from-cookie'); + $this->assertRaw("session_id:\n", t('Session ID is blank as sent from cookie header.')); + // Assert that we have an anonymous session now. + $this->drupalGet('session-test/is-logged-in'); + $this->assertResponse(403, t('An empty session ID is not allowed.')); + } + + /** * Reset the cookie file so that it refers to the specified user. * * @param $uid User id to set as the active session. diff --git a/modules/simpletest/tests/session_test.module b/modules/simpletest/tests/session_test.module index bf6c302e2..61aa3ae95 100644 --- a/modules/simpletest/tests/session_test.module +++ b/modules/simpletest/tests/session_test.module @@ -17,6 +17,12 @@ function session_test_menu() { 'access arguments' => array('access content'), 'type' => MENU_CALLBACK, ); + $items['session-test/id-from-cookie'] = array( + 'title' => 'Session ID from cookie', + 'page callback' => '_session_test_id_from_cookie', + 'access arguments' => array('access content'), + 'type' => MENU_CALLBACK, + ); $items['session-test/set/%'] = array( 'title' => 'Set session value', 'page callback' => '_session_test_set', @@ -49,6 +55,12 @@ function session_test_menu() { 'access arguments' => array('access content'), 'type' => MENU_CALLBACK, ); + $items['session-test/is-logged-in'] = array( + 'title' => 'Check if user is logged in', + 'page callback' => '_session_test_is_logged_in', + 'access callback' => 'user_is_logged_in', + 'type' => MENU_CALLBACK, + ); return $items; } @@ -104,6 +116,13 @@ function _session_test_id() { } /** + * Menu callback: print the current session ID as read from the cookie. + */ +function _session_test_id_from_cookie() { + return 'session_id:' . $_COOKIE[session_name()] . "\n"; +} + +/** * Menu callback, sets a message to me displayed on the following page. */ function _session_test_set_message() { @@ -165,3 +184,10 @@ function session_test_drupal_goto_alter(&$path, &$options, &$http_response_code) $path = $base_insecure_url . '/' . $path; } } + +/** + * Menu callback, only available if current user is logged in. + */ +function _session_test_is_logged_in() { + return t('User is logged in.'); +} diff --git a/modules/system/system.install b/modules/system/system.install index 94c0bb2ee..ad76e11be 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -2911,7 +2911,10 @@ function system_update_7065() { 'length' => 128, 'not null' => TRUE, ); - db_change_field('sessions', 'sid', 'sid', $spec); + db_drop_primary_key('sessions'); + db_change_field('sessions', 'sid', 'sid', $spec, array('primary key' => array('sid', 'ssid'))); + // Delete any sessions with empty session ID. + db_delete('sessions')->condition('sid', '')->execute(); } /** |