FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.10] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) by hartwork · Pull Request #139532 · python/cpython · GitHub

/ cpython Public

[3.10] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) - #139532

Merged
pablogsal merged 9 commits into
python:3.10from
hartwork:backport-f04bea4-3.10
Nov 25, 2025
Merged

[3.10] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234)#139532
pablogsal merged 9 commits into
python:3.10from
hartwork:backport-f04bea4-3.10

Conversation

hartwork commented Oct 3, 2025
edited by picnixz
Loading

Copy link
Copy Markdown
Contributor

Expose the XML Expat 2.7.2 mitigation APIs to disallow use of disproportional amounts of dynamic memory from within an Expat parser (see CVE-2025-59375 for instance).

The exposed APIs are available on Expat parsers, that is, parsers created by xml.parsers.expat.ParserCreate(), as:

  • parser.SetAllocTrackerActivationThreshold(threshold), and
  • parser.SetAllocTrackerMaximumAmplification(max_factor).

(cherry picked from commits f04bea4 and 68a1778)

CC @picnixz

picnixz and others added 4 commits October 3, 2025 01:42
…2025-59375) (python#139234)

Expose the XML Expat 2.7.2 mitigation APIs to disallow use of
disproportional amounts of dynamic memory from within an Expat
parser (see CVE-2025-59375 for instance).

The exposed APIs are available on Expat parsers, that is,
parsers created by `xml.parsers.expat.ParserCreate()`, as:

- `parser.SetAllocTrackerActivationThreshold(threshold)`, and
- `parser.SetAllocTrackerMaximumAmplification(max_factor)`.

(cherry picked from commit f04bea4)
…on API (python#139366)

Fix some typos left in f04bea4,
and simplify some internal functions to ease maintenance of future
mitigation APIs.

(cherry picked from commit 68a1778)
Comment thread Modules/pyexpat.c Outdated
hartwork requested a review from picnixz October 3, 2025 15:29

picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the backport!

Comment thread Modules/pyexpat.c Outdated
#include <stdbool.h>
#include "structmember.h" // PyMemberDef
#include "frameobject.h"
#include <stddef.h> // offsetof()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do we need offsetof?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@picnixz we do!

For one, because it is used in:

static PyMemberDef xmlparse_members[] = {
    {"intern", T_OBJECT, offsetof(xmlparseobject, intern), READONLY, NULL},
    {NULL}
};

For two, because it is added in de690f1, the source of the backport.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hum. Why did add it? I think it was because my IDE complained at some point. Ok then.

hartwork Oct 3, 2025
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@picnixz I need to correct me earlier statement: I got the commit wrong, it's actually f04bea4 and you did not add that import there, it was already present in main and 3.14 and 3.13. So the question now seems to be whether we want to add or not add that import in the backports targetting 3.12, 3.11 and 3.10. It's the correct header but then some other header must be already pulling it in or the code would not compile already prior to these backports. I'm good with dropping or keeping that new include — what would you prefer?

picnixz Oct 3, 2025
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Try to reduce the diff as much as possible. If it works without the include then let's not add it. I think it would make better sense to remove it from 3.13 and 3.14 actually but I don't know if other files actually have an explicit or an implicit include policy...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Try to reduce the diff as much as possible. If it works without the include then let's not add it.

@picnixz okay, let me try…

I think it would make better sense to remove it from 3.13 and 3.14 actually but I don't know if other files actually have an explicit or an implicit include policy...

That I would have trouble supporting. Please don't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@picnixz PS: I found…

Doc/c-api/structures.rst:   (You may need to ``#include <stddef.h>`` for :c:func:`!offsetof`.)

…and also…

Include/structmember.h:#include <stddef.h> /* For offsetof (not always provided by Python.h) */

in 3.12 code and that all of 3.10, 3.11, 3.12 have pyexpat.c include structmember.h but 3.13, 3.14, main do not and so these seem to rightfully contain a dedicated #include <stddef.h> for offsetof.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@picnixz okay, let me try…

Update: dropped from all of 3.10, 3.11, 3.12 backports by now.

picnixz commented Oct 7, 2025

Copy link
Copy Markdown
Member

To have a good synchronization, we'll also delay 3.10 to 3.13 backports for their next release cycle (see #139359 (comment)).

picnixz self-assigned this Oct 7, 2025

ambv commented Oct 8, 2025

Copy link
Copy Markdown
Contributor

I set DO-NOT-MERGE to avoid confusion. Unset that when you think we should be releasing this.

hartwork commented Nov 5, 2025

Copy link
Copy Markdown
Contributor Author

@pablogsal I believe this is ready to be merged. Do you have a minute?

picnixz commented Nov 8, 2025

Copy link
Copy Markdown
Member

This was this PR I wanted to check manually because we didn't have _Py_ID() access in 3.10

Copy link
Copy Markdown
Contributor Author

@pablogsal do you have a minute for this? 🙏

pablogsal merged commit 1173f80 into python:3.10 Nov 25, 2025
15 checks passed

Copy link
Copy Markdown
Member

Thanks a lot for the backport and thanks for the patience :)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL