Opened 2 days ago
Last modified 2 days ago
#36651 closed Bug
Security concerrn in ModelBackend — at Initial Version
Reported by: | heindrickdumdum0217 | Owned by: | |
---|---|---|---|
Component: | contrib.auth | Version: | 5.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
https://github.com/django/django/blob/main/django/contrib/auth/backends.py#L71
def authenticate(self, request, username=None, password=None, **kwargs): if username is None: username = kwargs.get(UserModel.USERNAME_FIELD) if username is None or password is None: return try: user = UserModel._default_manager.get_by_natural_key(username) except UserModel.DoesNotExist: # Run the default password hasher once to reduce the timing # difference between an existing and a nonexistent user (#20760). UserModel().set_password(password) else: if user.check_password(password) and self.user_can_authenticate(user): return user
We have implemented user account lock after 3 consecutive failed login attempts.
When user try to login in 4-th item we have to show correct error message about user account is locked, but for now it's impossible without rewriting "authenticate" function again.
But the current code checks password first, then check user can authenticate.
If means if user receives different error message, user can sure at least username and password are correct.
It may allow hackers can try with different password as many as times until they receive different error message.
For example, when password is not match, it returns error message unable to login with provided credentials.
But when acount is locked, it returns error message "your account is locked".
This is just an example.
What I'm going to say is we should check if user can authenticate first after get user from username or email.
Then compare password, otherwise it may allow hackers guess password.
Of course I can inherit ModelBackend class and update "authenticate" function, but I don't think it's a good approach.
When we inhert, we should use at least super class's function not override, because the "authenticate" function can be updated later in next Django releases.