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

Fix open redirect of "next" request parameter #150

Closed
wants to merge 2 commits into from

Conversation

bbc2
Copy link
Contributor

@bbc2 bbc2 commented Nov 16, 2016

This fixes the security issue reported in #119 by forcing the network location of the next parameter to be the same as that of the website.

@LotosikRa
Copy link
Contributor

LotosikRa commented Nov 25, 2016

Hi @bbc2 ,

I think that it would be good if you will make fallback parameter editable throw configuration (class-based or UserManager).

UPDATE: remove unneeded "for all use-cases"

@bbc2
Copy link
Contributor Author

bbc2 commented Nov 25, 2016

To be honest, I should probably not have introduced the fallback argument in the first place. This safe redirect mechanism is just aimed at users messing with the URLs. It's not a feature, just a defense. Am I missing some good use-case for a user-configurable fallback URL?

@lingthio What do you think? Should I keep this as is, remove the unneeded fallback argument, or make it configurable?

@LotosikRa
Copy link
Contributor

@bbc2 one good use-case is when your login-page (or another one) isn't on '/', or you want to warn a user about not-safe URL.

@bbc2
Copy link
Contributor Author

bbc2 commented Nov 29, 2016

OK, fair enough. I guess it would be a good candidate for an ENDPOINT parameter such as those listed in https://pythonhosted.org/Flask-User/customization.html#endpoints.

The weakness is also known as CWE 601.  This commit prevents open
redirects by checking every "next" parameter, either from a URL or a
form.

Closes lingthio#119
@bbc2
Copy link
Contributor Author

bbc2 commented Nov 30, 2016

There are two commits now. One for fixing the vulnerability, the other making the fallback URL configurable. Looks good to you?

@LotosikRa
Copy link
Contributor

@bbc2 thank you.
But fallback argument in safe_redirect function was good - it allows a developer to configure a fallback endpoint for his custom view function.

@bbc2
Copy link
Contributor Author

bbc2 commented Dec 1, 2016

I think it's easy to add if it turns out to be necessary. I'd like to keep it minimal.

@lingthio
Copy link
Owner

This feature got implemented a while back using the make_safe_url() function that can be customized. It's in Flask-User v0.6 as well as v1.0.

@lingthio lingthio closed this Apr 19, 2018
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

Successfully merging this pull request may close these issues.

3 participants