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

Security problem with `request.access_route` #41

Closed
QuantumGhost opened this issue Oct 12, 2015 · 4 comments

Comments

@QuantumGhost
Copy link

@QuantumGhost QuantumGhost commented Oct 12, 2015

According to the documentation of werkzeug, request.access_route is computed by looking at forwarded header in the request. However, as malicious users can submit request with X-Forwarded-For of any value, they can bypass limiters easily.

I think that this security problem should be documented clearly, and utils.get_ipaddr should be used with caution.

Here is an article discussing this problem: http://esd.io/blog/flask-apps-heroku-real-ip-spoofing.html.

@alisaifee

This comment has been minimized.

Copy link
Owner

@alisaifee alisaifee commented Oct 15, 2015

Thanks, you're right. Do you think using request.remote_addr as the value of the request's ip address would be a safer approach. I'm thinking of issuing a warning for anyone using the utils.get_ipaddr method and deprecating it over time. The documentation will then point to a recipe of how to get the ip address of the request with/without a proxy.

@tonymillion

This comment has been minimized.

Copy link

@tonymillion tonymillion commented Feb 19, 2016

👍 for this - I've just dealt with the same issue

Even though we used the werkzeug ProxyFix middleware, the default ratelimiter get_ipaddr was bypassing that and using a spoofed address (request.access_route[0]).

I'd suggest putting a big warning about using ProxyFix if you're behind a loadbalancer (in our case AWS ELB) and change the get_ipaddr to use request.remote_add

@alisaifee

This comment has been minimized.

Copy link
Owner

@alisaifee alisaifee commented Feb 29, 2016

@tonymillion & @QuantumGhost thank you for the feedback.

I've finally added the deprecation and some documentation about the problem here. The code remains backward compatible for now, but I will eventually remove the default key function behaviour.

Do let me know if the documentation I've added can be improved.

@marians

This comment has been minimized.

Copy link

@marians marians commented Jun 29, 2016

The change to ProxyFix, as explained by http://esd.io/blog/flask-apps-heroku-real-ip-spoofing.html, has landed in Werkzeug 0.11.10 and is available to users of the most recent Flask version. I suggest to extend the docs to show and explain the example

from werkzeug.contrib.fixers import ProxyFix
...
app.wsgi_app = ProxyFix(app.wsgi_app, num_proxies=1)

That increases the likelihood that developers of applications behind proxies pick up this important piece of information.

alisaifee added a commit that referenced this issue Oct 23, 2017
Addresses #41
@alisaifee alisaifee closed this Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.