lynndylanhurley / devise_token_auth

Token based authentication for Rails JSON APIs. Designed to work with jToker and ng-token-auth.
Do What The F*ck You Want To Public License
3.53k stars 1.13k forks source link

reset password flow leaves site vulnerable to phishing attacks #1454

Open trein-homeroom opened 3 years ago

trein-homeroom commented 3 years ago

The reset password flow puts a redirect_url in the password link that the user receives in email. This parameter can be changed by an attacker, and use it to send emails to people with a link that points to your site and then redirects to their site. Example:

I don't understand the motivation for putting the redirect_url parameter in the password reset link. Why isn't this a configuration parameter? If there is a valid reason for this being in reset link, then the token needs to also include signing all the URL parameters, and then you can verify the URL hasn't been tampered with.

If you disagree that this isn't a legit attack vector, I'll just say that I've already had multiple 3rd party "researchers" (hackers looking for $$) reach out to me about this issue on our site.

We're using version 1.1.3. I apologize if this has been fixed already, but I tried to examine the issues and the source code and it still looked like an issue.

trein-homeroom commented 3 years ago

I just saw that there's a whitelist ability, that would address this issue so you can close if you'd like. I didn't see that mentioned in the docs, I think you should call it out. But also still don't understand the why the redirect is in the URL to begin with and not just a config (and still also feel like the URL should be signed to be tamper proof).

lucasrogeriomasotti commented 3 years ago

I just realized the same thing you describe here today and I am very concerned as well. I think the redirect_whitelist option is helpful in preventing attacks, but the default configuration is not safe and the documentation does not mention redirect_whitelist very well, it is just another option on a list. The average user may skip pass it and have a security risk.

My suggestion is that to redirect_url to work, redirect_whitelist must be present, otherwise it should be ignored and the default redirect will be used. This would be a much safer and sane default config in my opinion.