Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Issue: Users in other browsers still logged-in after password has changed #824

Open
q2apro opened this issue Jun 19, 2020 · 7 comments

Comments

@q2apro
Copy link

q2apro commented Jun 19, 2020

From https://www.question2answer.org/qa/84833/security-bug-in-q2a

when the user changes his password his account still logged in another browser. If someone hacked my password there is no way to protect the account from hackers. There should be a option to log out of all sessions.

As a fix, I thought it is enough to change the sessioncode in table qa_users so it mismatches the sessioncode saved in the user cookie (qa_session). However, it doesn't work.

Any other idea?

@svivian
Copy link
Collaborator

svivian commented Jul 6, 2020

Spent some time digging into this, and there are a few complications. In PHP you can only access session data for the current visitor (as in, that IP address and browser). You can't get the other sessions for security reasons (a shared server would be able to access all your sessions).

In theory this shouldn't matter because sessions are supposed to expire after some time, or when you close your web browser. The exception is when you tick "remember me" - a session code is set in a cookie matching qa_users.sessioncode that keeps you logged in. The session code is removed when you change your password - this means all other sessions should be invalidated. However, there are two problems:

  • PHP does not delete sessions immediately when they expire, they are "garbage collected" at a very low rate (default is 0.1%). In fact I was not able to get it to delete sessions at all, even setting the rate higher.
  • These days browsers do not delete session cookies when you close the browser, if you have it set to continue where you left off.

So in the end I think the solution is that we should verify qa_users.sessioncode in the database each time. That way, when you change your password we can change the sessioncode and immediately log out all the other sessions.

@q2apro
Copy link
Author

q2apro commented Jul 7, 2020

Wouldn't it mean an extra query? Where would you add the code? In function qa_is_logged_in() I suppose.

@pupi1985
Copy link
Contributor

pupi1985 commented Jul 7, 2020

I need to understand this security issue a bit more. This all started with:

If someone hacked my password

So technically, it is already too late (we could even end discussion here). Anyway, let's assume this is step 1.

Step 2 is realizing that someone is using my account as well.

Step 3 would be making use of the "log out other sessions" feature.

Let's see the same scenario from the hacker point of view.

Step 1 is getting the password (somehow it is a success).

Step 2 is using the "log out other sessions" feature to make sure I have the only session and change the password.

The question is: who will click the "log out other sessions" first? The logical answer is the hacker.

There are better approaches to improve security (e.g. https://en.wikipedia.org/wiki/Multi-factor_authentication). Or, actually, that should be implemented first.

Is it only me who understands this in this way?

@q2apro
Copy link
Author

q2apro commented Jul 7, 2020

If the hacker changess the password, the user will use the password recovery. By using the pw link and creating a new password, the hacker will still be logged-in in his browser. To my understanding.

@pupi1985
Copy link
Contributor

pupi1985 commented Jul 7, 2020

I guess, all the hacker needs to do is to change the email address, as well.

@svivian
Copy link
Collaborator

svivian commented Jul 7, 2020

Wouldn't it mean an extra query? Where would you add the code?

The code would go in qa_get_logged_in_userid(). Q2A already fetches the user account on every page so there shouldn't be any more overhead.

So technically, it is already too late

Perhaps, but there are other situations such as using a public computer and forgetting to sign out, or your password being hacked on a different site so you change your password on this site.

Regardless of the above it would be more secure anyway to check both the PHP session and cookie match a valid user (currently only one of those is checked).

@q2apro
Copy link
Author

q2apro commented Jul 7, 2020

If there is no extra query (overhead), and not other potential risk or draw back, I would vote for the implementation.

By the way, I also faced the issue with an "anonymize account" plugin that I wrote. After the account has been anonymized the user account (by action of the user) I log out the user. However, in another browser, the user was still logged in and could see how the plugin renamed the user account, and he could still act within the forum! ... So I ended up to also block the user account with the plugin to prevent this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants