[Django] #37016: Avoid propagating invalid arguments from When() to Q() on dictionary expansion

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: 6.0 | Severity: Normal
Keywords: not-security, | Triage Stage:
_connector, _negated | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
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 swallow
`_connector` and `_negated` arguments instead of passing them down to
`Q()`, like this:

{{{#!py
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/37016>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: not-security, | Triage Stage:
_connector, _negated | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Jacob Walls:

Old description:

> 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 swallow
> `_connector` and `_negated` arguments instead of passing them down to
> `Q()`, like this:
>
> {{{#!py
> 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.

New description:

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:

{{{#!py
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.

--

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

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: not-security, | Triage Stage: Accepted
_connector, _negated |

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

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

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Varun
Type: | Kasyap Pentamaraju
Cleanup/optimization | Status: assigned

Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: not-security, | Triage Stage: Accepted
_connector, _negated |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Varun Kasyap Pentamaraju):

* owner: (none) => Varun Kasyap Pentamaraju
* status: new => assigned

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

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Varun
Type: | Kasyap Pentamaraju
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: not-security, | Triage Stage: Accepted
_connector, _negated |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Varun Kasyap Pentamaraju):

* has_patch: 0 => 1

Comment:

https://github.com/django/django/pull/21046
--
Ticket URL: <https://code.djangoproject.com/ticket/37016#comment:4>

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Varun
Type: | Kasyap Pentamaraju
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: not-security, | Triage Stage: Accepted
_connector, _negated |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37016#comment:5>

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Varun
Type: | Kasyap Pentamaraju
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: not-security, | Triage Stage: Ready for
_connector, _negated | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37016#comment:6>

[Django]

#37016: Avoid propagating invalid arguments from When() to Q() on dictionary
expansion
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Varun
Type: | Kasyap Pentamaraju

Cleanup/optimization | Status: closed

Component: Database layer | Version: 6.0
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: not-security, | Triage Stage: Ready for
_connector, _negated | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"3b161e60964aff99eddcd2627a486d81c1836b3a" 3b161e6]:
{{{#!CommitTicketReference repository=""
revision="3b161e60964aff99eddcd2627a486d81c1836b3a"
Fixed #37016 -- Avoided propagating invalid arguments from When() to Q().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37016#comment:7>