[Django] #31077: Add a safeguard to debug decorators (sensitive_variables/sensitive_post_parameters) to prevent incorrect usage

6 views
Skip to first unread message

Django

unread,
Dec 9, 2019, 6:35:15 AM12/9/19
to django-...@googlegroups.com
#31077: Add a safeguard to debug decorators
(sensitive_variables/sensitive_post_parameters) to prevent incorrect usage
------------------------------------------------+--------------------------
Reporter: Baptiste Mispelon | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+--------------------------
While trying to reproduce ticket:26480#comment:5, I noticed that Django
happily lets you write this kind of code:

{{{
#!python
@sensitive_variables # incorrect usage, should be @sensitive_variables()
def is_password_ok(password):
return len(password) > 8
}}}

It's very easy to miss that you forgot the `()`. Most of the time it's not
really dangerous because the decorated function will be unusable but in
this case, the consequences are pretty nasty:

{{{
#!python
>>> bool(is_password_ok('asdf'))
True # you would expect False because len('asdf') < 8
}}}

I propose adding some code to both `sensitive_variables()` and
`sensitive_post_parameters()` that catches this misuse to prevent users
from decorating their functions incorrectly.
Because both decorators take either no arguments or only string arguments,
it's not too hard to detect the error with something like this:
{{{
#!python
def sensitive_variables(*variables):
if len(variables) == 1 and callable(variables[0]):
raise TypeError(...)
# ...
}}}

This should be fully backwards compatible and in most cases it will raise
the error at import time which should make things easier to fix for those
who've incorrectly used the decorator.


(I've confirmed with the security team that this does not need to be
treated as a security issue)

--
Ticket URL: <https://code.djangoproject.com/ticket/31077>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 9, 2019, 6:38:39 AM12/9/19
to django-...@googlegroups.com
#31077: Add a safeguard to debug decorators
(sensitive_variables/sensitive_post_parameters) to prevent incorrect usage
-------------------------------------+-------------------------------------

Reporter: Baptiste Mispelon | Owner: (none)
Type: | Status: assigned
Cleanup/optimization |

Component: Error reporting | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Baptiste Mispelon):

[https://github.com/django/django/pull/12196 PR here]

I've included tests but no release notes. Does this need any?

--
Ticket URL: <https://code.djangoproject.com/ticket/31077#comment:1>

Django

unread,
Dec 9, 2019, 6:58:23 AM12/9/19
to django-...@googlegroups.com
#31077: Add a safeguard to debug decorators
(sensitive_variables/sensitive_post_parameters) to prevent incorrect usage.
--------------------------------------+------------------------------------

Reporter: Baptiste Mispelon | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by felixxm):

* stage: Unreviewed => Accepted


Comment:

Release notes are not required.

--
Ticket URL: <https://code.djangoproject.com/ticket/31077#comment:2>

Django

unread,
Dec 10, 2019, 7:58:51 AM12/10/19
to django-...@googlegroups.com
#31077: Add a safeguard to debug decorators
(sensitive_variables/sensitive_post_parameters) to prevent incorrect usage.
-------------------------------------+-------------------------------------
Reporter: Baptiste Mispelon | Owner: Mariusz
Type: | Felisiak <felisiak.mariusz@…>
Cleanup/optimization | Status: closed

Component: Error reporting | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* owner: (none) => Mariusz Felisiak <felisiak.mariusz@…>
* resolution: => fixed


Comment:

In [changeset:"d8e233352877c37c469687287e7761e05bdae94e" d8e23335]:
{{{
#!CommitTicketReference repository=""
revision="d8e233352877c37c469687287e7761e05bdae94e"
Fixed #31077 -- Made debug decorators raise TypeError if they're not
called.

Django will raise an error if you forget to call the decorator.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31077#comment:3>

Reply all
Reply to author
Forward
0 new messages