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

Bruteforce throttling vulnerability #151

Closed
wants to merge 1 commit into from

Conversation

noguespi
Copy link

Using HTTP_X_FORWARDED_FOR in the key is useless because the client can set a random X-Forwarded-For and bypass throttling.
Secondly using email in the key is useless too because there is little chance the same IP needs to access multiples accounts and have lost its passwords for all its accounts.

The rare case when one IP need to access multiple accounts is when it's behind a reverse proxy. In this case the 'HTTP_X_FORWARDED_FOR' should be used if it's properly set by the reverse proxy and not overridable.

Most of the time web application aren't behind reverse proxy so it's safe to only check REMOTE_ADDR

Using HTTP_X_FORWARDED_FOR in the key is useless because the client can set a random X-Forwarded-For and bypass throttling.
Secondly using email in the key is useless too because there is little chance the same IP needs to access multiples accounts and have lost its passwords for all its accounts.

The rare case when one IP need to access multiple accounts is when it's behind a reverse proxy. In this case the 'HTTP_X_FORWARDED_FOR' should be used if it's properly set by the reverse proxy and not overridable. 

Most of the time web application aren't behind reverse proxy so it's safe to only check REMOTE_ADDR
@andrew13
Copy link
Collaborator

What happens when someone is behind a reverse proxy then?

@noguespi
Copy link
Author

I think you mean someone behind a proxy and not a reverse proxy because only a server can be behind a reverse proxy (and all the client in front of the reverse proxy so in this case we use HTTP_X_FORWARDED_FOR instead of REMOTE_ADDR, because in this case most of the time we own the reverse proxy so we can trust the x-forward header). But like I said, most of the time server aren't behind reverse proxy.

But if someone is using a proxy, well the REMOTE_ADDR will be used, if multiple people connect from the same proxy and have loggin error, they will be all throttled. But that's not like it's going to happen a lot, it's a specific case. But If you keep X_FORWARDED_FOR in your session key, the throttle protection is useless because hacker can just spoof x-forwarded-for with random().

@noguespi
Copy link
Author

By the way, you don't have to ban the IP, to avoid false positive and limit bruteforce, just display a captcha

@andrew13
Copy link
Collaborator

Yeah captcha would be the best choice and there's plans to add it. Just haven't gotten around to doing so.

@andrew13
Copy link
Collaborator

Now I just need to figure out why the build is failing, and it'll get merged.

@andrew13
Copy link
Collaborator

Addressed this in: da0f75e

@andrew13 andrew13 closed this Oct 17, 2013
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.

None yet

2 participants