[ss-2017-006] Fix user agent invalidation on session startup

This commit is contained in:
Damian Mooyman 2017-11-30 14:48:36 +13:00
parent b31b22ac8e
commit 25e276cf37
2 changed files with 45 additions and 11 deletions

View File

@ -145,15 +145,7 @@ class Session {
if($data instanceof Session) $data = $data->inst_getAll(); if($data instanceof Session) $data = $data->inst_getAll();
$this->data = $data; $this->data = $data;
$this->expireIfInvalid();
if (isset($this->data['HTTP_USER_AGENT'])) {
if ($this->data['HTTP_USER_AGENT'] != $this->userAgent()) {
// Funny business detected!
$this->inst_clearAll();
$this->inst_destroy();
$this->inst_start();
}
}
} }
/** /**
@ -392,6 +384,9 @@ class Session {
$this->data = isset($_SESSION) ? $_SESSION : array(); $this->data = isset($_SESSION) ? $_SESSION : array();
} }
// Ensure session is validated on start
$this->expireIfInvalid();
// Modify the timeout behaviour so it's the *inactive* time before the session expires. // Modify the timeout behaviour so it's the *inactive* time before the session expires.
// By default it's the total session lifetime // By default it's the total session lifetime
if($timeout && !headers_sent()) { if($timeout && !headers_sent()) {
@ -631,4 +626,31 @@ class Session {
Deprecation::notice('4.0', 'Use the "Session.timeout" config setting instead'); Deprecation::notice('4.0', 'Use the "Session.timeout" config setting instead');
return Config::inst()->get('Session', 'timeout'); return Config::inst()->get('Session', 'timeout');
} }
/**
* Validate the user agent against the current data, resetting the
* current session if a mismatch is detected.
*
* @deprecated 3.0..4.0 Removed in 4.0
* @return bool If user agent has been set against this session, returns
* the valid state of this session as either true or false. If the agent
* isn't set it is assumed valid and returns true.
*/
private function expireIfInvalid() {
// If not set, indeterminable; Assume true as safe default
if (!isset($this->data['HTTP_USER_AGENT'])) {
return true;
}
// Agents match, deterministically true
if ($this->data['HTTP_USER_AGENT'] === $this->userAgent()) {
return true;
}
// Funny business detected!
$this->inst_clearAll();
$this->inst_destroy();
$this->inst_start();
return false;
}
} }

View File

@ -97,15 +97,27 @@ class SessionTest extends SapphireTest {
$_SERVER['HTTP_USER_AGENT'] = 'Test Agent'; $_SERVER['HTTP_USER_AGENT'] = 'Test Agent';
// Generate our session // Generate our session
/** @var Session $s */
$s = Injector::inst()->create('Session', array()); $s = Injector::inst()->create('Session', array());
$s->inst_set('val', 123); $s->inst_set('val', 123);
$s->inst_finalize(); $s->inst_finalize();
$data = $s->inst_getAll();
// Change our UA // Change our UA
$_SERVER['HTTP_USER_AGENT'] = 'Fake Agent'; $_SERVER['HTTP_USER_AGENT'] = 'Fake Agent';
// Verify the new session reset our values // Verify the new session reset our values (passed by constructor)
$s2 = Injector::inst()->create('Session', $s); /** @var Session $s2 */
$s2 = Injector::inst()->create('Session', $data);
$this->assertNotEquals($s2->inst_get('val'), 123); $this->assertNotEquals($s2->inst_get('val'), 123);
// Verify a started session resets our values (initiated by $_SESSION object)
/** @var Session $s3 */
$s3 = Injector::inst()->create('Session', []);
foreach ($data as $key => $value) {
$s3->inst_set($key, $value);
}
$s3->inst_start();
$this->assertNotEquals($s3->inst_get('val'), 123);
} }
} }