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 concern - Implementing limited retries ? #100

Open
SylvainLosey opened this issue Jan 3, 2022 · 3 comments
Open

Security concern - Implementing limited retries ? #100

SylvainLosey opened this issue Jan 3, 2022 · 3 comments

Comments

@SylvainLosey
Copy link

Hello there,

First thanks for the great package. I have been implementing it in a project, but I was surprised not to find a mechanism to limit retries. With a 6 digit token there is exactly a million possible combination - which seems easy to brute force in 15 minutes.

Is there a mechanism I missed to prevent these types of attacks ? If not, would you be open to a PR implementing the functionality ?

Bests,
Sylvain

@aaronn
Copy link
Owner

aaronn commented Jan 4, 2022

This sounds like a good idea, but I think if we do it we should use DRF's throttle policy somehow instead of building our own rate limiting mechanism.

@jws
Copy link
Contributor

jws commented Jan 4, 2022

given that throttling for non-trival cases generally require some sort of shared backend (i.e. redis), you should rely on (and really, need) the underlying framework's throttling capabilities to be configured and enabled. there might be some setting at drfpasswordless level for ease of customization - but you can't just have that.

to limit retries - you may be able to get by with enabling rest_framework.throttling.AnonRateThrottle as one of the DRF DEFAULT_THROTTLE_CLASSES and setting the rate to something like 'anon' : '10/minute'. that will apply rate limiting over drfpasswordless APIs.

@neilbags
Copy link

I did some testing and yes it is possible to brute-force the 6-digit code. This should be mentioned in the README, as the configuration described there is insecure.

Using AnonRateThrottle won't work, as it looks at the X-Forwarded-For header which an attacker can spoof. A better solution is to rate-limit /auth/token based on the email address or mobile number in the request, rather than the source IP address. This can be done with django-ratelimit. This solution does create a denial-of-service vulnerability, where an attacker could lock out a valid user.

This library is also vulnerable to account enumeration. The responses from /auth/email and /auth/mobile specify whether an account exists or not which is a concern in many applications. I modified the code so that the same response was sent regardless however I found that it was still possible to enumerate accounts based on response time. Rate-limiting /auth/email and /auth/mobile helps but does not eliminate this vulnerability.

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

No branches or pull requests

4 participants