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

Mitigating attacks using stolen reset password tokens #5281

Open
eliotsykes opened this issue Aug 17, 2020 · 5 comments
Open

Mitigating attacks using stolen reset password tokens #5281

eliotsykes opened this issue Aug 17, 2020 · 5 comments

Comments

@eliotsykes
Copy link

eliotsykes commented Aug 17, 2020

There is a window of opportunity for an attacker who has infiltrated access to server logs to takeover the accounts of anyone trying to reset their password:

  1. Victim visits GET /password/edit?reset_password_token=MY_TOKEN
  2. GET /password/edit?reset_password_token=MY_TOKEN is written to server log
  3. Attacker exfiltrates token from server log
  4. Attacker visits GET /password/edit?reset_password_token=MY_TOKEN
  5. Attacker submits new password at POST /password and takes over victim's account. (Note this step needs to happen quickly, before the victim submits their new password. Attacker could script this process to improve success rate.)
  6. Victim tries to submit a new password but fails. They are told their reset token expired and they need to restart at the forgot password form.

I'm looking for feedback on this possible mitigation and for other suggestions:

Expire the original token by replacing it with a fresh token at step 1 above, as soon as the intended victim requests GET /password/edit?reset_password_token=ORIGINAL_TOKEN.

The fresh token is then rendered in the new password form as a hidden field, replacing the original token.

<input type=hidden name=user[reset_password_token] value=FRESH_TOKEN>

The fresh token is never rendered in a URL querystring. The fresh token would never be sent in an HTTP GET request, and so wouldn't be written to the server log.

(NB. Generating the fresh token probably should not update the reset_password_sent_at datetime so the token lifetime isn't extended needlessly).

When the attacker tries to steal the original token from the server log, it is too late. The attacker visits GET /password/edit?reset_password_token=ORIGINAL_TOKEN but the token won't be recognized as it has been replaced.

The intended victim POSTs the password reset form, with the fresh token and their new password, and they successfully change their password without their account being taken over.


What issues are there with the above mitigation? What other mitigations should be considered?

@alecvn
Copy link

alecvn commented Jun 10, 2021

@eliotsykes did you implement this mitigation? It's a little discouraging that you never received any feedback on this issue.

@eliotsykes
Copy link
Author

No feedback yet unfortunately. I will add a thought shortly about this for anyone considering implementing it.

@eliotsykes
Copy link
Author

What issues are there with the above mitigation?

One potential issue to be aware of is users who visit the reset URL more than once. For example:

  1. User clicks password reset link in Gmail app (or some other mail client with a built-in browser) on their phone
  2. Link is opened in the Gmail app's built-in browser. Original token is replaced.
  3. User clicks button to open the link in the device's default browser (on iOS this opens Safari). This will use the original token which no longer works!
  4. User cannot reset password in device's default browser.

So using the strategy suggested in this issue may cause an increase in customer support queries about password resets.

@carlosantoniodasilva
Copy link
Member

Hey, apologies here, I've been very slow at going back at previous issues.

@eliotsykes appreciate the detailed report. This is certainly a concern, but I wouldn't categorize it as a huge one since it requires access to another piece of data that should be very well guarded as well: the logs.

What you just described above about the link being usable only once and replaced as soon as the request hits the server is definitely a problem people will run into, and it's the first thing that crossed my mind. It might be a trade off between that usability concern, and the security issue that this may or may not pose.

There's also a similar concern with HEAD requests, see this issue in relation to the confirmation flow/token: #5312, but I think that applies to reset password too, and something Devise should probably improve at some point. (e.g. we wouldn't want some random HEAD request to hit the server and replace the token so the user can never really reset their password in that case)

I'm not sure what the right response is, it certainly needs some balance between usability and security here, I'll think about it more, but please feel free to throw any other thoughts here and we can keep the discussion going.

@eliotsykes
Copy link
Author

eliotsykes commented Jun 10, 2021

Thanks @carlosantoniodasilva, no apology necessary, I really appreciate your work on devise and understand there's only so much time in the day.

Perhaps the password reset token page refresh behavior could become configurable, with the current, more usable one being the default. Something like:

Devise.setup do |config|
  ...

  # Current default, allows for page refresh
  config.allow_password_reset_page_refresh = true

  # Less usable, does not allow page refresh, mitigates token leaking from logs
  config.allow_password_reset_page_refresh = false

  ...
end

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

No branches or pull requests

3 participants