Opened 4 years ago

Closed 4 years ago

#31077 closed Cleanup/optimization (fixed)

Add a safeguard to debug decorators (sensitive_variables/sensitive_post_parameters) to prevent incorrect usage.

Reported by: Baptiste Mispelon Owned by: Mariusz Felisiak <felisiak.mariusz@…>
Component: Error reporting Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

While trying to reproduce ticket:26480#comment:5, I noticed that Django happily lets you write this kind of code:

@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:

>>> 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:

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)

Change History (3)

comment:1 by Baptiste Mispelon, 4 years ago

PR here

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

comment:2 by Mariusz Felisiak, 4 years ago

Summary: Add a safeguard to debug decorators (sensitive_variables/sensitive_post_parameters) to prevent incorrect usageAdd a safeguard to debug decorators (sensitive_variables/sensitive_post_parameters) to prevent incorrect usage.
Triage Stage: UnreviewedAccepted

Release notes are not required.

comment:3 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Owner: set to Mariusz Felisiak <felisiak.mariusz@…>
Resolution: fixed
Status: assignedclosed

In d8e23335:

Fixed #31077 -- Made debug decorators raise TypeError if they're not called.

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

Note: See TracTickets for help on using tickets.
Back to Top