From d8a860c95efc103b1bcc986f304e0f6c3eb72234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jer=C3=B4me=20Bakker?= Date: Thu, 6 Oct 2022 12:01:04 +0200 Subject: [PATCH] security(session): invalidate other sessions on user password change When a user changes their password all other sessions of that user should be invalidated. https://cwe.mitre.org/data/definitions/613.html --- engine/classes/ElggSession.php | 56 +++++++++++++++++++ engine/classes/ElggUser.php | 5 ++ engine/lib/sessions.php | 13 +++++ .../Elgg/Application/BootHandlerUnitTest.php | 1 + .../phpunit/unit/ElggSessionUnitTest.php | 33 ++++++++++- 5 files changed, 107 insertions(+), 1 deletion(-) diff --git a/engine/classes/ElggSession.php b/engine/classes/ElggSession.php index f22b6d8874c..e0c7d2cace7 100644 --- a/engine/classes/ElggSession.php +++ b/engine/classes/ElggSession.php @@ -267,6 +267,62 @@ public function removeLoggedInUser() { $this->remove('guid'); _elgg_services()->sessionCache->clear(); } + + /** + * Set a user specific token in the session for the currently logged in user + * + * This will invalidate the session on a password change of the logged in user + * + * @param \ElggUser $user the user to set the token for (default: logged in user) + * + * @return void + * @since 3.3.25 + */ + public function setUserToken(\ElggUser $user = null): void { + if (!$user instanceof \ElggUser) { + $user = $this->getLoggedInUser(); + } + if (!$user instanceof \ElggUser) { + return; + } + + $this->set('__user_token', $this->generateUserToken($user)); + } + + /** + * Validate the user token stored in the session + * + * @param \ElggUser $user the user to check for + * + * @return void + * @throws SecurityException + * @since 3.3.25 + */ + public function validateUserToken(\ElggUser $user): void { + $session_token = $this->get('__user_token'); + $user_token = $this->generateUserToken($user); + + if ($session_token !== $user_token) { + throw new SecurityException(elgg_echo('session_expired')); + } + } + + /** + * Generate a token for a specific user + * + * @param \ElggUser $user the user to generate the token for + * + * @return string + * @since 3.3.25 + */ + protected function generateUserToken(\ElggUser $user): string { + $hmac = _elgg_services()->hmac->getHmac([ + $user->time_created, + $user->guid, + ], 'sha256', $user->password_hash); + + return $hmac->getToken(); + } /** * Get current ignore access setting. diff --git a/engine/classes/ElggUser.php b/engine/classes/ElggUser.php index 5b151868021..3ca79920fbd 100644 --- a/engine/classes/ElggUser.php +++ b/engine/classes/ElggUser.php @@ -495,6 +495,11 @@ public function canComment($user_guid = 0, $default = null) { */ public function setPassword($password) { $this->setMetadata('password_hash', _elgg_services()->passwords->generateHash($password)); + if ($this->guid === elgg_get_logged_in_user_guid()) { + // update the session user token, so this session remains valid + // other sessions for this user will be invalidated + _elgg_services()->session->setUserToken(); + } } /** diff --git a/engine/lib/sessions.php b/engine/lib/sessions.php index df2e30d0422..12669126917 100644 --- a/engine/lib/sessions.php +++ b/engine/lib/sessions.php @@ -265,6 +265,7 @@ function login(\ElggUser $user, $persistent = false) { // use elgg_get_logged_in_user_entity(). $session = elgg()->session; $session->setLoggedInUser($user); + $session->setUserToken(); // re-register at least the core language file for users with language other than site default elgg()->translator->registerTranslations(\Elgg\Project\Paths::elgg() . 'languages/'); @@ -390,6 +391,8 @@ function _elgg_session_boot(ServiceProvider $services) { $user = $services->persistentLogin->bootSession(); if ($user) { $services->persistentLogin->updateTokenUsage($user); + // make sure the session is valid as there is no login + $session->setUserToken($user); } } @@ -402,6 +405,16 @@ function _elgg_session_boot(ServiceProvider $services) { logout(); return false; } + + // make sure the user hasn't changed in between requests + // e.g. password reset + try { + $session->validateUserToken($user); + } catch (\SecurityException $e) { + register_error($e->getMessage()); + logout(); + return false; + } } $services->timer->end([__FUNCTION__]); diff --git a/engine/tests/phpunit/unit/Elgg/Application/BootHandlerUnitTest.php b/engine/tests/phpunit/unit/Elgg/Application/BootHandlerUnitTest.php index 86cf700833e..27212571f8e 100644 --- a/engine/tests/phpunit/unit/Elgg/Application/BootHandlerUnitTest.php +++ b/engine/tests/phpunit/unit/Elgg/Application/BootHandlerUnitTest.php @@ -156,6 +156,7 @@ public function testCanSetCustomUserClassDuringBootSequence() { ]); $app->_services->session->set('guid', $user->guid); + $app->_services->session->setUserToken($user); // normally this is done during login() $user->invalidateCache(); diff --git a/engine/tests/phpunit/unit/ElggSessionUnitTest.php b/engine/tests/phpunit/unit/ElggSessionUnitTest.php index 512cbd782fe..80bbd443106 100644 --- a/engine/tests/phpunit/unit/ElggSessionUnitTest.php +++ b/engine/tests/phpunit/unit/ElggSessionUnitTest.php @@ -14,7 +14,8 @@ public function up() { } public function down() { - + _elgg_services()->session->removeLoggedInUser(); + _elgg_services()->session->invalidate(); } public function testStart() { @@ -61,4 +62,34 @@ public function testCanSetLoggedInUser() { $this->assertNull($session->getLoggedInUser()); } + public function testUserTokenValidation() { + $user = $this->createUser(); + $session = \ElggSession::getMock(); + + $session->setUserToken($user); + $session->validateUserToken($user); + + // change the user password + $user->setPassword('some new password'); + + // token isn't valid anymore + $this->expectException(\SecurityException::class); + $session->validateUserToken($user); + } + + public function testUserTokenValidationLoggedIn() { + $user = $this->createUser(); + $session = _elgg_services()->session; + + $session->setLoggedInUser($user); + $session->setUserToken(); + + $session->validateUserToken($user); + + // change the user password + $user->setPassword('some new password'); + + // session should remain valid + $session->validateUserToken($user); + } }