Commit 96dc4766 authored by Dries's avatar Dries

- Patch #280934 by pwolanin, swentel, et al: harden session regeneration. It...

- Patch #280934 by pwolanin, swentel, et al: harden session regeneration.  It took a while, but it comes with tests and extra features now.
parent bd955495
......@@ -163,6 +163,9 @@ function _sess_write($key, $value) {
*/
function drupal_session_regenerate() {
$old_session_id = session_id();
extract(session_get_cookie_params());
// Set "httponly" to TRUE to reduce the risk of session stealing via XSS.
session_set_cookie_params($lifetime, $path, $domain, $secure, TRUE);
session_regenerate_id();
db_update('sessions')
->fields(array(
......
......@@ -7,6 +7,9 @@
*/
class SessionTestCase extends DrupalWebTestCase {
protected $saved_cookie;
/**
* Implementation of getInfo().
*/
......@@ -24,16 +27,61 @@ class SessionTestCase extends DrupalWebTestCase {
function setUp() {
parent::setUp('session_test');
}
/**
* Implementation of curlHeaderCallback().
*/
protected function curlHeaderCallback($ch, $header) {
// Look for a Set-Cookie header.
if (preg_match('/^Set-Cookie.+$/i', $header, $matches)) {
$this->saved_cookie = $header;
}
return parent::curlHeaderCallback($ch, $header);
}
/**
* Tests for drupal_save_session().
* Tests for drupal_save_session() and drupal_session_regenerate().
*/
function testSessionSaveSession() {
function testSessionSaveRegenerate() {
$this->assertTrue(drupal_save_session(), t('drupal_save_session() correctly returns TRUE when initially called with no arguments.'), t('Session'));
$this->assertFalse(drupal_save_session(FALSE), t('drupal_save_session() correctly returns FALSE when called with FALSE.'), t('Session'));
$this->assertFalse(drupal_save_session(), t('drupal_save_session() correctly returns FALSE when saving has been disabled.'), t('Session'));
$this->assertTrue(drupal_save_session(TRUE), t('drupal_save_session() correctly returns TRUE when called with TRUE.'), t('Session'));
$this->assertTrue(drupal_save_session(), t('drupal_save_session() correctly returns TRUE when saving has been enabled.'), t('Session'));
// Test session hardening code from SA-2008-044.
$user = $this->drupalCreateUser(array('access content'));
// Enable sessions.
$this->sessionReset($user->uid);
// Make sure the session cookie is set as HttpOnly.
$this->drupalLogin($user);
$this->assertTrue(preg_match('/HttpOnly/i', $this->saved_cookie), t('Session cookie is set as HttpOnly.'));
$this->drupalLogout();
// Verify that the session is regenerated if a module calls exit
// in hook_user_login().
user_save($user, array('name' => 'session_test_user'));
$user->name = 'session_test_user';
$this->drupalGet('session-test/id');
$matches = array();
preg_match('/\s*session_id:(.*)\n/', $this->drupalGetContent(), $matches);
$this->assertTrue(!empty($matches[1]) , t('Found session ID before logging in.'));
$original_session = $matches[1];
// We cannot use $this->drupalLogin($user); because we exit in
// session_test_user_login() which breaks a normal assertion.
$edit = array(
'name' => $user->name,
'pass' => $user->pass_raw
);
$this->drupalPost('user', $edit, t('Log in'));
$this->drupalGet('node');
$pass = $this->assertText($user->name, t('Found name: %name', array('%name' => $user->name)), t('User login'));
$this->_logged_in = $pass;
$this->drupalGet('session-test/id');
$matches = array();
preg_match('/\s*session_id:(.*)\n/', $this->drupalGetContent(), $matches);
$this->assertTrue(!empty($matches[1]) , t('Found session ID after logging in.'));
$this->assertTrue($matches[1] != $original_session, t('Session ID changed after login.'));
}
/**
......
......@@ -11,6 +11,12 @@ function session_test_menu() {
'access arguments' => array('access content'),
'type' => MENU_CALLBACK,
);
$items['session-test/id'] = array(
'title' => t('Session ID value'),
'page callback' => '_session_test_id',
'access arguments' => array('access content'),
'type' => MENU_CALLBACK,
);
$items['session-test/set/%'] = array(
'title' => t('Set Session value'),
'page callback' => '_session_test_set',
......@@ -58,3 +64,22 @@ function _session_test_no_set($value) {
_session_test_set($value);
return t('session saving was disabled, and then %val was set', array('%val' => $value));
}
/**
* Menu callback: print the current session ID.
*/
function _session_test_id() {
return 'session_id:' . session_id() . "\n";
}
/**
* Implementation of hook_user().
*/
function session_test_user_login($edit = array(), $user = NULL) {
if ($edit['name'] == 'session_test_user') {
// Exit so we can verify that the session was regenerated
// before hook_user() was called.
exit;
}
}
......@@ -1361,8 +1361,11 @@ function user_authenticate_finalize(&$edit) {
// This is also used to invalidate one-time login links.
$user->login = REQUEST_TIME;
db_query("UPDATE {users} SET login = %d WHERE uid = %d", $user->login, $user->uid);
user_module_invoke('login', $edit, $user);
// Regenerate the session ID to prevent against session fixation attacks.
// This is called before hook_user in case one of those functions fails
// or incorrectly does a redirect which would leave the old session in place.
drupal_session_regenerate();
user_module_invoke('login', $edit, $user);
}
/**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment