[Django] #36912: Q.create() doesn't validate connectors as Q.__init__() does

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob | Owner: Sarah Boyce
Walls |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords: _connector security
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
In 98e642c69181c942d60a10ca0085d48c6b3068bb, we mitigated a SQL injection
vector for user-controlled arguments to `filter()` and friends
(CVE-2025-64459) by adding validation for the `_connector` argument.

We deliberately avoided adding the same validation to `Q.create()`,
because `Q.create` is an undocumented internal not to be used with user-
controlled input.

The Security Team then received more than one report extrapolating from
CVE-2025-64459, suggesting that `Q.create` was missing the same
validation.

Although we don't consider this a security vulnerability, we would be
interested to evaluate if adding validation to `Q.create` to match
`Q.__init__` would be cheap enough to implement. A concern is how many
times `Q.create` might be called in loops. Thus, one part of the solution
here might be a refactor to reduce the number of times `Q.create` is
called in loops in Django's code:

in `contrib.contenttypes`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/contrib/contenttypes/fields.py#L682-L695
in the deletion `Collector`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/db/models/deletion.py#L357-L358
...others?
----
Tentatively passing over to Djangonaut Space navigators to see if a good
fit for anyone (git archaeology, benchmarking, security, ORM internals)
--
Ticket URL: <https://code.djangoproject.com/ticket/36912>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Sarah
Type: | Boyce
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: _connector security | Triage Stage:

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

> In 98e642c69181c942d60a10ca0085d48c6b3068bb, we mitigated a SQL injection
> vector for user-controlled arguments to `filter()` and friends
> (CVE-2025-64459) by adding validation for the `_connector` argument.
>
> We deliberately avoided adding the same validation to `Q.create()`,
> because `Q.create` is an undocumented internal not to be used with user-
> controlled input.
>
> The Security Team then received more than one report extrapolating from
> CVE-2025-64459, suggesting that `Q.create` was missing the same
> validation.
>
> Although we don't consider this a security vulnerability, we would be
> interested to evaluate if adding validation to `Q.create` to match
> `Q.__init__` would be cheap enough to implement. A concern is how many
> times `Q.create` might be called in loops. Thus, one part of the solution
> here might be a refactor to reduce the number of times `Q.create` is
> called in loops in Django's code:
>
> in `contrib.contenttypes`:
> https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/contrib/contenttypes/fields.py#L682-L695
> in the deletion `Collector`:
> https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/db/models/deletion.py#L357-L358
> ...others?
> ----
> Tentatively passing over to Djangonaut Space navigators to see if a good
> fit for anyone (git archaeology, benchmarking, security, ORM internals)

New description:

In 98e642c69181c942d60a10ca0085d48c6b3068bb, we mitigated a SQL injection
vector for user-controlled arguments to `filter()` and friends
(CVE-2025-64459) by adding validation for the `_connector` argument.

We deliberately avoided adding the same validation to `Q.create()`,
because `Q.create` is an undocumented internal not to be used with user-

controlled field names.

The Security Team then received more than one report extrapolating from
CVE-2025-64459, suggesting that `Q.create` was missing the same
validation.

Although we don't consider this a security vulnerability, we would be
interested to evaluate if adding validation to `Q.create` to match
`Q.__init__` would be cheap enough to implement. A concern is how many
times `Q.create` might be called in loops. Thus, one part of the solution
here might be a refactor to reduce the number of times `Q.create` is
called in loops in Django's code:

in `contrib.contenttypes`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/contrib/contenttypes/fields.py#L682-L695
in the deletion `Collector`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/db/models/deletion.py#L357-L358
...others?
----
Tentatively passing over to Djangonaut Space navigators to see if a good
fit for anyone (git archaeology, benchmarking, security, ORM internals)

--

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

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Sarah
Type: | Boyce
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage:
| 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:

> In 98e642c69181c942d60a10ca0085d48c6b3068bb, we mitigated a SQL injection
> vector for user-controlled arguments to `filter()` and friends
> (CVE-2025-64459) by adding validation for the `_connector` argument.
>
> We deliberately avoided adding the same validation to `Q.create()`,
> because `Q.create` is an undocumented internal not to be used with user-

> controlled field names.
>
> The Security Team then received more than one report extrapolating from
> CVE-2025-64459, suggesting that `Q.create` was missing the same
> validation.
>
> Although we don't consider this a security vulnerability, we would be
> interested to evaluate if adding validation to `Q.create` to match
> `Q.__init__` would be cheap enough to implement. A concern is how many
> times `Q.create` might be called in loops. Thus, one part of the solution
> here might be a refactor to reduce the number of times `Q.create` is
> called in loops in Django's code:
>
> in `contrib.contenttypes`:
> https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/contrib/contenttypes/fields.py#L682-L695
> in the deletion `Collector`:
> https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/db/models/deletion.py#L357-L358
> ...others?
> ----
> Tentatively passing over to Djangonaut Space navigators to see if a good
> fit for anyone (git archaeology, benchmarking, security, ORM internals)

New description:

In 98e642c69181c942d60a10ca0085d48c6b3068bb, we mitigated a SQL injection
vector for user-controlled arguments to `filter()` and friends
(CVE-2025-64459) by adding validation for the `_connector` argument.

We deliberately avoided adding the same validation to `Q.create()`,
because `Q.create` is an undocumented internal not to be used with user-

controlled field names and was created specifically for the purpose of
speed.

The Security Team then received more than one report extrapolating from
CVE-2025-64459, suggesting that `Q.create` was missing the same
validation.

Although we don't consider this a security vulnerability, we would be
interested to evaluate if adding validation to `Q.create` to match
`Q.__init__` would be cheap enough to implement. A concern is how many
times `Q.create` might be called in loops. Thus, one part of the solution
here might be a refactor to reduce the number of times `Q.create` is
called in loops in Django's code:

in `contrib.contenttypes`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/contrib/contenttypes/fields.py#L682-L695
in the deletion `Collector`:
https://github.com/django/django/blob/13299a6203f4bc3e5b2552c96a51ff2b15da3c43/django/db/models/deletion.py#L357-L358
...others?
----
Tentatively passing over to Djangonaut Space navigators to see if a good
fit for anyone (git archaeology, benchmarking, security, ORM internals)

--
--

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

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Sarah
Type: | Boyce
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: _connector security | Triage Stage: Accepted

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

Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted

Comment:

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

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Sarah
Type: | Boyce
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Anna Makarudze):

Hey, I am Anna. I would like to work on this as part of the Djangonaut 6
Session (Team Saturn).
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:4>

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Anna
Type: | Makarudze

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

Changes (by Jacob Walls):

* owner: Sarah Boyce => Anna Makarudze

Comment:

Terrific, I'm looking forward to your findings.
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:5>

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Anna
Type: | Makarudze
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Anna Makarudze):

Hey Jacob, just an update.

I was having a discussion with Tim Schilling, and he suggested that I
investigate the following:

Is the original reason for Q.create() still valid? The docstring on
tree.Node.create() indicates the signatures were different between
Q.__init__() and Q.create() hence the reason for the extra method. It
looks like they are slightly different, but the biggest difference is in
the unpacking and the sort() call.

Is there a performance difference if we swap out all Q.create() usages
with Q() instead. This could be determined by creating a commit to Django
that uses that (confirm the tests pass), then using that with django-asv
to compare performance

What is the performance impact of calling [*args, *sorted(kwargs.items())]
in Q.__init__()? This could be calculated in a script. This is where AI
can be helpful. I used Claude to generate a script that provided a
framework to tweak options to better understand the impact of that:
https://claude.ai/share/d7af3307-58c8-4c91-af82-0d30d0571dfc

I investigated the 2nd scenario only with django-asv and the benchmarks
indicated better performance for Q() than Q.create(). I didn't go ahead
with creating a commit to Django using that method. I will be
investigating 1 and 3 this week. Any thoughts or insights on how to tackle
this ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:6>

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Anna
Type: | Makarudze
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

Hi Anna, thanks for these details. I agree the unpacking is the main
difference, and that was the main rationale for switching to `Q.create()`
more in 9dff316be41847c505ebf397e4a52a0a71e0acc4. From that commit
message:

> In addition, we were often needing to unpack iterables into *args and
can instead pass a list direct to Node.create().

In other words, we already had an iterable, so we were able to micro-
optimize out the unpacking by using `Q.create()`.

Maybe we could have our cake and eat it too by just overriding `create()`
on `Q` to do the necessary checking of valid connectors. This would allow
us to get the desired checking without the (possibly) undesired cost of
unpacking and sorting/repacking.

> I investigated the 2nd scenario only with django-asv and the benchmarks
indicated better performance for Q() than Q.create().

(At first glance, this seems surprising to me -- it's worth a second look
to ensure your bench is fully isolating what you wanted to measure.)

Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:7>

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Anna
Type: | Makarudze
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

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

Changes (by Anna Makarudze):

* has_patch: 0 => 1

Comment:

I have two pull requests open, one for Django here
https://github.com/django/django/pull/21022 and another for django-asv
here https://github.com/django/django-asv/pull/98. I am familiar with
benchmarks so hopefully I got them right this time.
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:8>

[Django]

#36912: Q.create() doesn't validate connectors as Q.__init__() does
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Anna
Type: | Makarudze
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: _connector security | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

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

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

Comment:

Thank you Anna
I commented on the django-asv benchmark PR. I think the next step would be
to share the results of running the benchmarks against main and then
against main + your commit
--
Ticket URL: <https://code.djangoproject.com/ticket/36912#comment:9>