Opened 4 weeks ago

Closed 4 weeks ago

Last modified 3 weeks ago

#36651 closed Bug (invalid)

Brute-force password attack against inactive users returns distinct error message

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 (last modified by heindrickdumdum0217)

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 account is locked, but for now it's impossible without rewriting "authenticate" function again.

The current code checks password first, then check user can authenticate.
It means if user receives different error message, attacker 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.

Change History (5)

comment:1 by heindrickdumdum0217, 4 weeks ago

Description: modified (diff)

comment:2 by Jacob Walls, 4 weeks ago

Resolution: invalid
Status: newclosed

As mentioned in the ticket submission form, security-related reports are not to be submitted here. They should be sent to security@… instead.

That said, we do not consider this a security issue. If the user is active, brute-forcing the password results in successful authentication. The techniques to protect against this are well-known, including requiring strong passwords and rate-limiting requests to authentication endpoints.

Here you have raised the case where the user is inactive and authentication does not succeed, but the correctness of the password can be inferred from the variance in the error. But in the active user case, it's already "game over" if the password can be brute-forced. We wouldn't add complexity to treat the inactive user case differently. Moreover, reversing the order of conditions could cause an account enumeration attack, see #20760.

comment:3 by Jacob Walls, 4 weeks ago

Summary: Security concerrn in ModelBackendBrute-force password attack against inactive users returns distinct error message

in reply to:  2 comment:4 by heindrickdumdum0217, 4 weeks ago

Replying to Jacob Walls:

As mentioned in the ticket submission form, security-related reports are not to be submitted here. They should be sent to security@… instead.

That said, we do not consider this a security issue. If the user is active, brute-forcing the password results in successful authentication. The techniques to protect against this are well-known, including requiring strong passwords and rate-limiting requests to authentication endpoints.

Here you have raised the case where the user is inactive and authentication does not succeed, but the correctness of the password can be inferred from the variance in the error. But in the active user case, it's already "game over" if the password can be brute-forced. We wouldn't add complexity to treat the inactive user case differently. Moreover, reversing the order of conditions could cause an account enumeration attack, see #20760.

Hi Jacob
Thanks for your comment.

Brute-forcing doesn't work for active user, becuase after 3 consecutive failed login attempts, account will be locked.
Of course we have implemented rate-limit too.

But even though we have rate-limit, it's still less secure, because hackers can try with different passwords until they reach rate-limit for inactive user.
If we check if user can authenticate first, we can prevent password guessing even hackers try with different passwords until they reach rate-limit.

Last edited 4 weeks ago by heindrickdumdum0217 (previous) (diff)

comment:5 by heindrickdumdum0217, 3 weeks ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top