close
The Wayback Machine - https://web.archive.org/web/20200706010620/https://github.com/getsentry/sentry/issues/11907
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

Prevent Referer leaking on password reset page #11907

Open
mattrobenolt opened this issue Feb 5, 2019 · 7 comments · May be fixed by #14913
Open

Prevent Referer leaking on password reset page #11907

mattrobenolt opened this issue Feb 5, 2019 · 7 comments · May be fixed by #14913

Comments

@mattrobenolt
Copy link
Member

@mattrobenolt mattrobenolt commented Feb 5, 2019

Right now when you get a password reset link, the page you end up on contains the reset token in the url.

Doing this causes a problem in which any page redirected to after this, and especially any third party assets being loaded leak this reset token via the Referer header on the request.

While this isn't specifically a problem since the only things we embed are trusted vendors, in the case of SaaS, it's better to just avoid it.

My proposal is to keep the same url, but this endpoint does the following:

  • Accepts the request, and puts the token either into the browser session or browser's cookie.
  • Redirect away from this url, to a generic password reset form page.
  • This new password reset form page then takes the values from your session or cookie.

Doing this mitigates any ability for leaking data through any Referer header in the future.

I am open to other proposals, but this is the best that comes to my mind.

@dcramer dcramer added the Easy Task label Feb 5, 2019
@Kacppian
Copy link

@Kacppian Kacppian commented Feb 26, 2019

Can I work on this?

@mattrobenolt
Copy link
Member Author

@mattrobenolt mattrobenolt commented Feb 26, 2019

@Kacppian Sure!

@Kacppian
Copy link

@Kacppian Kacppian commented Mar 5, 2019

Hey, I have some doubts. Couldn't find a place to ask questions. I tried irc #sentry but couldn't find anyone to talk to or maybe I'm bad at using my irc client. Can you please help me?

On lostpasswordhash.py, lno - 68, I see the template of the email is for both set_password and recover_account. But grep reveals no service actually is using template as 'set_password'. May I know why? If this is the wrong way to reach out, I'm extremely sorry.

@Kacppian
Copy link

@Kacppian Kacppian commented Mar 5, 2019

My idea of solving this was to use recover_account view to set the token in the browser session and redirect to set_password view to be able to set the password without the hash in the URL. That led me to see if set_password template is being used

@markstory
Copy link
Member

@markstory markstory commented Mar 6, 2019

I see the template of the email is for both set_password and recover_account. But grep reveals no service actually is using template as 'set_password'. May I know why?

The set_password email is unused. It was added in e962e56 but doesn't appear to ever have been used. There is a set_password flow for when SSO is disabled on an organization but that is a separate thing.

Your plan of setting into the session and redirecting sounds like a good plan to me.

@le0pard
Copy link

@le0pard le0pard commented Sep 29, 2019

Hello.

I think to resolve this problem sentry server just need provide header Referrer-Policy with value strict-origin-when-cross-origin : https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy

@ezzy1337
Copy link

@ezzy1337 ezzy1337 commented Oct 1, 2019

I'd be happy to take a crack at this for Hacktoberfest!

bsr3000 added a commit to bsr3000/sentry that referenced this issue Oct 2, 2019
Add `Referrer-Policy` header with `strict-origin-when-cross-origin` value to responses on views with password reset hash.

Requests to third-party services on password reset page won't contain path with `hash` in referrer header

Fixes getsentryGH-11907
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.