[Django] #36769: Limit recursive extraction of field values in XML deserializer

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Core
| (Serialization)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
While investigating
[https://docs.djangoproject.com/en/5.2/releases/security/#december-2-2025-cve-2025-64460
CVE-2025-64460] (mitigated in 50efb718b31333051bc2dcb06911b8fa1358c98c),
we noticed that the private helper `getInnerText` supports extracting
[https://github.com/django/django/blob/93540b34d4ef46f68df2c8bfe90447d0f649a852/django/core/serializers/xml_serializer.py#L418
arbitrarily nested text], however its only use in Django is to extract
text at most one level deep, under a `<natural>` tag, like this
[https://github.com/django/django/blob/5625bd590766e5ca8c2c76ba2307b98f7450ff83/tests/fixtures/fixtures/fixture9.xml#L22
fixture example].

We opted not to change this semantic in a patch release, but it occurred
to me that we could only extract text at the exact expected depth (e.g. 0
if under `<field>` and 1 if under `<field><natural>`) and completely
sidestep potential performance issues from invalid input making use of
nested elements, see recent
[https://www.cve.org/CVERecord?id=CVE-2025-12084 python CVE-2025-12084] we
also mitigated yesterday.

I would appreciate any arguments I might be missing, for example, if there
are legitimate use cases for ingesting text from nested tags e.g. from
unescaped markup that this proposal would break.
--
Ticket URL: <https://code.djangoproject.com/ticket/36769>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:

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

Changes (by Pravin):

* cc: Pravin (added)

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

[Django]

#36769: Limit recursive extraction of field values in XML deserializer

--------------------------------------+------------------------------------

Reporter: Jacob Walls | Owner: (none)

Type: Cleanup/optimization | Status: new
Component: Core (Serialization) | Version: dev

Severity: Normal | Resolution:
Keywords: xml deserializer | 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):

* cc: Shai Berger (added)
* keywords: => xml deserializer
* stage: Unreviewed => Accepted

Comment:

Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:2>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
--------------------------------------+------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Pravin):

Either we could change the helper function to only accept the types that
Django actually uses: regular text and the one-level-deep <natural> tag.
This would stop the recursion immediately for anything else, or we could
explicitly tell the function how deep it should go.
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:3>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
--------------------------------------+------------------------------------

Reporter: Jacob Walls | Owner: Pravin
Type: Cleanup/optimization | Status: assigned

Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Pravin):

* owner: (none) => Pravin
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:4>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
--------------------------------------+------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: Cleanup/optimization | Status: assigned
Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

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

Changes (by Jacob Walls):

* needs_tests: 0 => 1

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

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
--------------------------------------+------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: Cleanup/optimization | Status: assigned
Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 1 | 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/36769#comment:6>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
--------------------------------------+------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: Cleanup/optimization | Status: assigned
Component: Core (Serialization) | Version: dev
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

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

* needs_tests: 1 => 0

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

[Django]

#36769: Limit recursive extraction of field values in XML deserializer

-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Pravin

Type: | Status: assigned

Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |

Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Ready for
| checkin

Has patch: 0 | 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/36769#comment:8>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: | Status: assigned
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution:
Keywords: xml deserializer | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

One advantage of merging this cleanup is that it hastens the day we can
remove the workarounds and the test from
1dbd07a608e495a0c229edaaf84d58d8976313b5, which we merged knowing might be
flaky (and indeed has been).

I'm subscribed to the [https://github.com/python/cpython/pull/142211
Python 3.12 backport PR] with the mitigation for the `minidom` issue.
Waiting for that to land and for a 3.12 patch release after.
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:9>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Pravin

Type: | Status: closed

Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |

Severity: Normal | Resolution: fixed

Keywords: xml deserializer | Triage Stage: Ready for
| checkin
Has patch: 0 | 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:"dae08cf55b83caef5e8ee39b16417692e8565278" dae08cf5]:
{{{#!CommitTicketReference repository=""
revision="dae08cf55b83caef5e8ee39b16417692e8565278"
Fixed #36769 -- Avoided visiting deeply nested nodes in XML deserializer.

Only children at one level of depth need to be visited.

Co-authored-by: Jacob Walls <jacobty...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:10>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: | Status: closed
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution: fixed
Keywords: xml deserializer | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

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

In [changeset:"73c5e94521c5b97e27cd2fe2d5b5c2e65f402755" 73c5e94]:
{{{#!CommitTicketReference repository=""
revision="73c5e94521c5b97e27cd2fe2d5b5c2e65f402755"
Refs #36769 -- Raised SuspiciousOperation for unexpected nested tags in
XML Deserializer.

Thanks Shai Berger and Natalia Bidart for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:13>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: | Status: closed
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution: fixed
Keywords: xml deserializer | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"1a70889d5837982a111bca0280ea6ca782c6de6b" 1a70889]:
{{{#!CommitTicketReference repository=""
revision="1a70889d5837982a111bca0280ea6ca782c6de6b"
Refs #36769 -- Corrected invalid XML fixtures.

fixture9.xml was likely wrong since its introduction in
35cc439228cd32dfa7a3ec919db01a8a5cd17d33.

The relevant part of the Visa model is:

class Visa(models.Model):
person = models.ForeignKey(Person, models.CASCADE)

The Visa.person <field>s needed to be declared as relations, and
the Person <field>s didn't need their values wrapped in `<natural>`,
since they weren't relations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:11>

[Django]

#36769: Limit recursive extraction of field values in XML deserializer
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Pravin
Type: | Status: closed
Cleanup/optimization |
Component: Core | Version: dev
(Serialization) |
Severity: Normal | Resolution: fixed
Keywords: xml deserializer | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"a25158f5cc590f3dff4226c3a48257481e6e67a6" a25158f5]:
{{{#!CommitTicketReference repository=""
revision="a25158f5cc590f3dff4226c3a48257481e6e67a6"
Refs #36769 -- Avoided visiting grandchild nodes in XML Deserializer.

The only use case for visiting grandchild nodes turned out to be to
support an unintentionally invalid fixture in the test suite.

The invalid fixture added in #36969 was modeled on fixture9.xml in
dae08cf55b83caef5e8ee39b16417692e8565278, so that is corrected as well
in this commit, where the test will still pass.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36769#comment:12>