#36709: Bug Report: `manage.py check` Fails to Detect `@staticmethod` Override
for
`is_authenticated` (auth.C010) and 'is_anonymous' (auth.C009)
---------------------------+----------------------------------------
Reporter: cyberstan | Type: Bug
Status: new | Component: contrib.auth
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
---------------------------+----------------------------------------
'''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
1. '''Define a `VulnerableUser` model in `models.py`:'''
{{{#!python
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
}}}
2. '''Point to this model in `settings.py`:'''
{{{
AUTH_USER_MODEL = 'myapp.VulnerableUser'
}}}
3. '''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:'''
{{{#!python
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:'''
{{{#!python
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36709>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019a51393a47-508d5061-c7b5-4532-8a2e-79797003a9eb-000000%40eu-central-1.amazonses.com.
