Commit f103c650 authored by Robert Lyon's avatar Robert Lyon

Session is not invalidating after password change (Bug #1363873)

Scenario/testing:

- Create an account, say User A and logout as admin.
- In one browser login (this will be the hacker user)
- In another browser reset pass via forgotten pass link

What should happen:
User in browser two should be able to reset pass then navigate about
as when normally logged in. User in browser one should be forced to
login again as their user sessionid is not valid anymore.

Before patch:
malicious user still has access until $USER->logout_time time expires

After patch:
malicious user foreced to re-login straight away on next page load

Change-Id: I42ad907e5ffa7c128742a159116cf20dc6cd9b8a
Signed-off-by: Robert Lyon's avatarRobert Lyon <robertl@catalyst.net.nz>
parent 364f87f6
......@@ -414,7 +414,13 @@ function auth_setup () {
// Check the time that the session is set to log out. If the user does
// not have a session, this time will be 0.
$sessionlogouttime = $USER->get('logout_time');
if ($sessionlogouttime && isset($_GET['logout'])) {
// Need to doublecheck that the User's sessionid still has a match the usr_session table
// It can disappear if the current user has hacked the real user's account and the real user has
// reset the password clearing the session from usr_session.
$sessionexists = get_record('usr_session', 'usr', $USER->id, 'session', $USER->get('sessionid'));
$parentuser = $USER->get('parentuser');
if (($sessionlogouttime && isset($_GET['logout'])) || ($sessionexists === false && $USER->get('sessionid') != '' && empty($parentuser))) {
// Call the authinstance' logout hook
$authinstance = $SESSION->get('authinstance');
if ($authinstance) {
......
......@@ -223,6 +223,10 @@ function forgotpasschange_submit(Pieform $form, $values) {
ensure_user_account_is_active($user);
$USER->reanimate($user->id, $user->authinstance);
// Destroy other sessions of the user
remove_user_sessions($USER->get('id'));
$SESSION->add_ok_msg(get_string('passwordchangedok'));
redirect();
exit;
......
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