Opened 4 hours ago
#36709 new Bug
Bug Report: `manage.py check` Fails to Detect `@staticmethod` Override for `is_authenticated` (auth.C010) and 'is_anonymous' (auth.C009)
| Reported by: | stan shaw | 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: | yes | UI/UX: | no |
Description
Summary:
The Django system check command (manage.py check) fails to identify an issue when a custom user model overrides is_authenticated or is_anonymous with a @staticmethod.
The relevant checks, auth.C009 and auth.C010, appear to check only for MethodType, but do not correctly identify staticmethod objects.
This is a flaw in the check's tooling, as this override pattern causes bool(User()) to incorrectly evaluate to True, which could lead to issues in custom developer code that manually instantiates the user model.
How to Reproduce
- Define a
VulnerableUsermodel inmodels.py:from django.contrib.auth.models import AbstractUser class VulnerableUser(AbstractUser): """ This model overrides is_authenticated with a staticmethod, which the check command fails to detect. """ @staticmethod def is_authenticated(): return False @staticmethod def is_anonymous(): return False
- Point to this model in
settings.py:AUTH_USER_MODEL = 'myapp.VulnerableUser'
- Run the system check:
python manage.py check
Expected Behavior
The check command should detect the improper override and report errors auth.C009 and auth.C010:
System check identified 2 issues: CRITICAL: myapp.VulnerableUser.is_anonymous must be an attribute or property... (auth.C009) CRITICAL: myapp.VulnerableUser.is_authenticated must be an attribute or property... (auth.C010)
Actual Behavior
The check command fails to detect the staticmethod and reports no issues:
System check identified no issues (0 silenced).
Proposed Patch
The vulnerability in the check lies in django/contrib/auth/checks.py.
The current check incorrectly uses isinstance(..., MethodType), which fails to detect staticmethod overrides as they are resolved as type function.
The check should be modified to use callable() instead. AnonymousUser (the correct implementation) uses @property, which results in a bool value, and callable(False) is False. The vulnerable staticmethod override results in a function object, and callable(<function>) is True.
File: django/contrib/auth/checks.py
Current (flawed) code:
if isinstance(cls().is_anonymous, MethodType): errors.append( checks.Critical( "%s.is_anonymous must be an attribute or property rather than " "a method. Ignoring this is a security issue as anonymous " "users will be treated as authenticated!" % cls, obj=cls, id="auth.C009", ) ) if isinstance(cls().is_authenticated, MethodType): errors.append( checks.Critical( "%s.is_authenticated must be an attribute or property rather " "than a method. Ignoring this is a security issue as anonymous " "users will be treated as authenticated!" % cls, obj=cls, id="auth.C010", ) ) return errors
Suggested patch:
if callable(cls().is_anonymous): # MODIFIED errors.append( checks.Critical( "%s.is_anonymous must be an attribute or property rather than " "a method. Ignoring this is a security issue as anonymous " "users will be treated as authenticated!" % cls, obj=cls, id="auth.C009", ) ) if callable(cls().is_authenticated): # MODIFIED errors.append( checks.Critical( "%s.is_authenticated must be an attribute or property rather " "than a method. Ignoring this is a security issue as anonymous " "users will be treated as authenticated!" % cls, obj=cls, id="auth.C010", ) ) return errors