aaronn / django-rest-framework-passwordless

Passwordless Auth for Django REST Framework
MIT License
708 stars 153 forks source link

Security concern - Implementing limited retries ? #100

Open SylvainLosey opened 2 years ago

SylvainLosey commented 2 years ago

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

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

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 commented 1 year ago

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.