Opened 3 weeks ago

Closed 3 weeks ago

#37016 closed Cleanup/optimization (fixed)

Avoid propagating invalid arguments from When() to Q() on dictionary expansion

Reported by: Jacob Walls Owned by: Varun Kasyap Pentamaraju
Component: Database layer (models, ORM) Version: 6.0
Severity: Normal Keywords: not-security, _connector, _negated
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jacob Walls)

The Security Team just closed a report extrapolating from 279f8b9557f0fef9790822b0c38164fc9dfcab2a arguing that When() is missing the same protection we gave to filter() and friends, whereby we raise errors for the _connector and _negated arguments instead of passing them down to Q(), like this:

    def _filter_or_exclude_inplace(self, negate, args, kwargs):
        if invalid_kwargs := PROHIBITED_FILTER_KWARGS.intersection(kwargs):
            invalid_kwargs_str = ", ".join(f"'{k}'" for k in sorted(invalid_kwargs))
            raise TypeError(f"The following kwargs are invalid: {invalid_kwargs_str}")

We don't consider this a vulnerability, as we didn't even consider 279f8b9557f0fef9790822b0c38164fc9dfcab2a a vulnerability, just an incidental finding we shipped with the security releases (notice the commit message says "Refs ..." not "Fixed ...") and thought prudent to backport. (The crux of the CVE was the arbitrary SQL injection, not a query logic bug downstream of a user's failure to validate user inputs.)

Still, we would welcome a PR to django's main branch that extends the protection quoted above to When().

Thanks m0_ld for the report.

Change History (7)

comment:1 by Jacob Walls, 3 weeks ago

Description: modified (diff)

comment:2 by Sarah Boyce, 3 weeks ago

Triage Stage: UnreviewedAccepted

comment:3 by Varun Kasyap Pentamaraju, 3 weeks ago

Owner: set to Varun Kasyap Pentamaraju
Status: newassigned

comment:4 by Varun Kasyap Pentamaraju, 3 weeks ago

Has patch: set

comment:5 by Jacob Walls, 3 weeks ago

Patch needs improvement: set

comment:6 by Jacob Walls, 3 weeks ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Jacob Walls <jacobtylerwalls@…>, 3 weeks ago

Resolution: fixed
Status: assignedclosed

In 3b161e6:

Fixed #37016 -- Avoided propagating invalid arguments from When() to Q().

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