lingthio / Flask-User

Customizable User Authorization & User Management: Register, Confirm, Login, Change username/password, Forgot password and more.
http://flask-user.readthedocs.io/
MIT License
1.06k stars 292 forks source link

Fix open redirect of "next" request parameter #150

Closed bbc2 closed 6 years ago

bbc2 commented 8 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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.

bbc2 commented 7 years ago

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

LotosikRa commented 7 years ago

@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 commented 7 years ago

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

lingthio commented 6 years ago

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.