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

Open redirect vulnerability #21

Closed
Guezone opened this issue Oct 22, 2020 · 2 comments · Fixed by #24
Closed

Open redirect vulnerability #21

Guezone opened this issue Oct 22, 2020 · 2 comments · Fixed by #24

Comments

@Guezone
Copy link

Guezone commented Oct 22, 2020

First of all, thank you for the work! For the detail, there is an Open Redirect vulnerability in flask_simplelogin when authenticating after trying to access a page where the @login_required directive is set. An attacker can then send a link to : https://goodsite.com/login/?next=https://badsite.com/login -> The user authenticates and is then redirected to the wrong site with the same appearance (potentially) indicating for example "login failed", he then retypes his credentials and that's it for the attacker...

I think it would be interesting to allow redirection only if the "next url" is "routable".

@cuducos
Copy link
Member

cuducos commented Oct 22, 2020

That's great. Just think out loud, a way to verify that in SimpleLogin.login method:

        if not any(r.match(destiny) for r in self.app.url_map.iter_rules())
            # TODO raise 404, specific error, or redirect do home?

@Guezone
Copy link
Author

Guezone commented Oct 22, 2020

Thank you very much !

@Guezone Guezone closed this as completed Oct 22, 2020
@cuducos cuducos reopened this Oct 22, 2020
rochacbruno added a commit that referenced this issue Aug 23, 2021
Based on @cuducos comments
@rochacbruno rochacbruno linked a pull request Aug 23, 2021 that will close this issue
rochacbruno added a commit that referenced this issue Aug 23, 2021
Based on @cuducos comments
cuducos pushed a commit that referenced this issue Aug 23, 2021
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 a pull request may close this issue.

2 participants