-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Ruff: Fix N805 #13437
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
Ruff: Fix N805 #13437
Conversation
|
This pull request introduces a potential authorization bypass in which the rate-limiting decorator can cause enable_force_password_reset to be invoked by unauthenticated attackers: by repeatedly attempting logins for different valid usernames an attacker can trigger the rate limit and force password reset for those accounts, effectively enabling denial-of-service and unauthorized security actions on other users' accounts.
Potential Authorization Bypass on Password Reset Flag in
|
| Vulnerability | Potential Authorization Bypass on Password Reset Flag |
|---|---|
| Description | The enable_force_password_reset method, when called from the rate-limiting decorator in dojo/decorators.py, can be triggered by an unauthenticated attacker. By repeatedly attempting to log in with different valid usernames, an attacker can cause the rate limit to be hit for those users, thereby forcing them to reset their passwords. This constitutes a denial of service for legitimate users and an authorization bypass, as an attacker can force a security action on another user's account without proper authorization. |
django-DefectDojo/dojo/models.py
Lines 231 to 245 in e84f748
| def force_password_reset(user): | |
| return hasattr(user, "usercontactinfo") and user.usercontactinfo.force_password_reset | |
| def disable_force_password_reset(self): | |
| if hasattr(self, "usercontactinfo"): | |
| self.usercontactinfo.force_password_reset = False | |
| self.usercontactinfo.save() | |
| def enable_force_password_reset(self): | |
| if hasattr(self, "usercontactinfo"): | |
| self.usercontactinfo.force_password_reset = True | |
| self.usercontactinfo.save() | |
| @staticmethod | |
| def generate_full_name(user): |
All finding details can be found in the DryRun Security Dashboard.
mtesauro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
|
@kiblik would it make sense to double check the (force) change password still works? |
Great point @valentijnscholten I really doubt we have a test for that flow. |
Well, I had not been able to test |
Fix N805 by making methods static or implementing them correctly.