reset password vulnerability

[♥raiwa]

I got the following reported:

Quote

Session does not get invalidated after password change

Description
Website doesn't invalidate session after the password is reset which can enable the attacker to continue using the compromised session.

Steps I followed
*I logged into the same account in two different browsers
*Change password in one browser and you will see that another browser still validates the session after password change (even after refreshing the page ).

Impact
If any user's account gets compromised and the user gets aware of it, he/she will try to change the password hoping that the account will be secured after changing the password for obvious reasons. But the case is not the same here as the attacker will still be able to use the compromised session even after the user changes his/her password. for this particular session management vulnerability.

Mitigation
When a user changes his/her password, all the active sessions that belong to that particular account must be destroyed.

I would like to recommend you to add a process that asks users whether the user wants to close all active sessions or not right after changing the password.

So there are multiple ways, either you let users choose if they want to keep active sessions or just destroy every active session when an user changes his/her password.

Matt @ecartz, provided the following script/hook to fix this:

class hook_shop_siteWide_reset_all_sessions {

  public function listen_accountUpdateTables($parameters) {
    if (isset($parameters['db']['customers']['customers_password'])) {
      $sessions_query = tep_db_query("DELETE s FROM sessions s INNER JOIN whos_online wo ON s.sesskey = wo.session_id WHERE wo.customer_id = " . (int)$_SESSION['customer_id']);
    }
  }

}

I made the hook which should be placed in:

Phoenix 1.0.7.2.+:
templates/default/includes/hooks/shop/siteWide/

Phoenix 1.0.5.1. - 1.0.7.1:
includes/hooks/shop/siteWide/

reset_all_sessions.php

 

Lower Phoenix and OSCommerce versions need to add the query to account_password.php line 49-50.

So it should look like this:

      if (tep_validate_password($password_current, $check_customer['customers_password'])) {
        tep_db_query("update customers set customers_password = '" . tep_encrypt_password($password_new) . "' where customers_id = '" . (int)$customer_id . "'");

        tep_db_query("update customers_info set customers_info_date_account_last_modified = now() where customers_info_id = '" . (int)$customer_id . "'");

// session destroy on password reset
        tep_db_query("DELETE s FROM sessions s INNER JOIN whos_online wo ON s.sesskey = wo.session_id WHERE wo.customer_id = " . (int)$customer_id );

        $messageStack->add_session('account', SUCCESS_PASSWORD_UPDATED, 'success');
        
        tep_redirect(tep_href_link('account.php', '', 'SSL'));

Matt asked me to publish this here so other users can test it before adding it to core.

It is already in use in one live store.

Please test and report back.

[René H4]

I must say that the shop owner or developer that found this issue has very well dugg into the problem! 

Well done, and thanks @raiwa and @ecartz for developing a solution!