GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.13] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) by hartwork · Pull Request #139367 · python/cpython

Conversation

@hartwork

@hartwork hartwork commented Sep 26, 2025 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

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 commit f04bea4)

CC @picnixz

  • Issue: Expose Expat XML attack protection APIs #90949

📚 Documentation preview 📚: https://cpython-previews--139367.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@picnixz @hartwork
pythongh-90949: add Expat API to prevent XML deadly allocations (CVE-…
5f9e8b8
…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)
@bedevere-app bedevere-app Bot added the awaiting review label Sep 26, 2025
This was referenced Sep 26, 2025
Expose Expat XML attack protection APIs #90949
Closed
gh-90949: add Expat API to prevent XML deadly allocations #139234
Merged
@bedevere-app bedevere-app Bot added the topic-XML label Sep 26, 2025
@picnixz

picnixz commented Sep 26, 2025

Copy link
Copy Markdown
Member

Ah I think we cross-posted but I actually want to simplify the future backports and next PR by using another generic handler (it will ease the diff). See #139366

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Sep 26, 2025

Copy link
Copy Markdown
Contributor Author

Ah I think we cross-posted but I actually want to simplify the future backports and next PR by using another generic handler (it will ease the diff). See #139366

@picnixz I saw your message #139234 (comment) but did not expect impact on my wip 3.13 backport and not more than that fix. What should best be done with this pull request? Closing is alright with me if automation replaces my work after merge of #139366 — can it? Else I'm happy to adjust in here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 26, 2025

Copy link
Copy Markdown
Member

Closing is alright with me if automation replaces my work after merge

I'll try to make an automated backport. If it succeeds, then good. Otherwise we'll need something else :( However, I'm surprised by the segfaults?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 26, 2025

Copy link
Copy Markdown
Member

(Segfaults could be because you didn't make clinic; and there are some directives that are not available in 3.13, see the 3.14 backport; maybe backporting the 3.14 PR would be easier actually)

hartwork reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
hartwork marked this pull request as draft September 26, 2025 17:03
@bedevere-app bedevere-app Bot removed the awaiting review label Sep 26, 2025
@hartwork

hartwork commented Sep 26, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz I'll wait for merge of #139366 and then get back to it. (It did not segfault on my local amd64.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 26, 2025

Copy link
Copy Markdown
Member

The address sanitizer says that there is a heap-UAF.

hartwork reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hartwork and others added 3 commits September 26, 2025 19:32
@hartwork
pyexpat.c: Drop @permit_long_* to make peace with "make clinic"
9357fd0
@hartwork
Apply "make clinic" with force
9dd807e
@picnixz @hartwork
pythongh-90949: amend pythonGH-139234 in prevision of future mitigati…
b235056
…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)
@hartwork

hartwork commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz I have used the same technique as #139359 (review) here now to find any potential issues in the diff backported. The key thing that stands out is the difference in xmlparseobject versus PyObject and related casts, but I don't see obvious trouble there from a superficial look.

I'm attaching my three diffs if you would like to see my cross-diff comparison with your own eyes:

  • diff-3-13.diff.txt
    i.e. the essence of [3.13] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) #139367 against 3.13 here
  • diff-3-14.diff.txt
    i.e. the essence of your [3.14] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) #139359 against 3.14
  • diff-main.diff.txt
    i.e. the essence of your gh-90949: add Expat API to prevent XML deadly allocations #139234 against main (or 3.15)

Looking from a different angle next…

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
Workaround use-after-free from *__fail_for_subparser tests for the mo…
cff9c7c
…ment
@hartwork

hartwork commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz I found that…

Two (or arguably four) tests are failing:

  • test_set_maximum_amplification__fail_for_subparser
  • test_set_activation_threshold__fail_for_subparser

A parser instance is being freed twice.

The minimal reproducer for Python 3.13 (but not 3.14) is this:

class UseAfterFreeCrashDemoTest(unittest.TestCase):
    def test_use_after_free__crash(self):
        parser = expat.ParserCreate()
        subparser = parser.ExternalEntityParserCreate(None)

While interestingly adding del subparser works around the problem:

class NoUseAfterFreeHereTest(unittest.TestCase):
    def test_use_after_free__no_crash(self):
        parser = expat.ParserCreate()
        subparser = parser.ExternalEntityParserCreate(None)
        del subparser

Similarly, appending…

        del setter
        del subparser
        del parser

…to both original tests in here works around the issue. My understanding is that there is an unrelated object graph issue here that makes CPython free things in the wrong order. I'll add my workaround for a green CI for the moment, create a new separate issue about that, and hope for ideas from your side about how to proceed from there.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 28, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

My understanding is that there is an unrelated object graph issue here that makes CPython free things in the wrong order

That's interesting. It could be the incremental GC which was deferred to Python 3.14 (https://docs.python.org/3.14/whatsnew/3.14.html#incremental-garbage-collection).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 28, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Anyway, we can correctly assume that we have a UAF. Let's create a separate issue (do you want to?)

hartwork reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork hartwork mentioned this pull request Sep 28, 2025
Heap-use-after-free issue in pyexpat related to .ExternalEntityParserCreate #139400
Closed
@hartwork

hartwork commented Sep 28, 2025

Copy link
Copy Markdown
Contributor Author

Anyway, we can correctly assume that we have a UAF. Let's create a separate issue (do you want to?)

@picnixz I was already at it: #139400

picnixz reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
hartwork marked this pull request as ready for review September 28, 2025 17:54
@bedevere-app bedevere-app Bot added the awaiting review label Sep 28, 2025
@hartwork

hartwork commented Sep 28, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@picnixz I have marked this PR here as ready for review now. It's the same as yours for 3.14 — #139359 — plus the workaround for #139400.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 28, 2025

Copy link
Copy Markdown
Member

I would prefer waiting for the UAF to be fixed if it's ok for you. We usually don't like having a branch with temporary workarounds.

hartwork reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Sep 28, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I would prefer waiting for the UAF to be fixed if it's ok for you. We usually don't like having a branch with temporary workarounds.

@picnixz good with me, I can revert the workaround later 👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork
hartwork marked this pull request as draft September 29, 2025 22:19
@bedevere-app bedevere-app Bot removed the awaiting review label Sep 29, 2025
@hartwork
Add note on the nature of the workaround
59a9613
@hartwork
hartwork marked this pull request as ready for review October 2, 2025 21:32
@bedevere-app bedevere-app Bot added the awaiting review label Oct 2, 2025
hartwork added 3 commits October 3, 2025 00:05
@hartwork
Add note on the nature of the workaround
ba2968b
@hartwork
Merge remote-tracking branch 'upstream/3.13' into backport-f04bea4-3.13
2a25a71
@hartwork
Drop workaround for 139400 as no-longer-needed
f6e0edc
@picnixz

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)).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz self-assigned this Oct 7, 2025
@ambv ambv added the DO-NOT-MERGE label Oct 8, 2025
@ambv

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.

picnixz reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz removed the DO-NOT-MERGE label Nov 2, 2025
picnixz
picnixz approved these changes Nov 2, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Nov 2, 2025
@picnixz

picnixz commented Nov 2, 2025

Copy link
Copy Markdown
Member

The commit message will be

* gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (#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 f04bea44c37793561d753dd4ca6e7cd658137553)
(cherry picked from commit 68a1778b7721f3fb853cd3aa674f7039c2a4df36)

I'll rerun the CI

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Nov 2, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@hartwork can you merge 3.13 into this backport please to trigger a CI run? and do the same for the other backports? (just hit the "update branch" button; I can't because you didn't allow maintainers to directly push on your branch)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hartwork added 2 commits November 2, 2025 13:10
@hartwork
Merge remote-tracking branch 'upstream/3.13' into backport-f04bea4-3.13
fc50ad5
@hartwork
Resolve double "textwrap" import to make linter happy
12d2b24
@hartwork

hartwork commented Nov 2, 2025

Copy link
Copy Markdown
Contributor Author

@picnixz done for 3.{10..13} just now 👍

picnixz reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz enabled auto-merge (squash) November 2, 2025 12:22
@picnixz

picnixz commented Nov 2, 2025

Copy link
Copy Markdown
Member

Thanks. I won't be able to merge 3.10, 3.11 and 3.12 as only the RMs can. I will review them myself because they weren't automated backports IIRC and we needed to tweak a bit things and there because of the lack of some functions (or maybe we did so in a smart way already). Anyway, I need to work on something else for today but hopefully I'll be able to review it by next week. Then, we can backport the other PRs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hartwork

hartwork commented Nov 2, 2025

Copy link
Copy Markdown
Contributor Author

Thanks. I won't be able to merge 3.10, 3.11 and 3.12 as only the RMs can.

@picnixz understood, yes 👍

I will review them myself because they weren't automated backports IIRC and we needed to tweak a bit things and there because of the lack of some functions (or maybe we did so in a smart way already).

Do you want me to summarize the difference between these four backports?

Anyway, I need to work on something else for today but hopefully I'll be able to review it by next week. Then, we can backport the other PRs.

Thank you! 👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz merged commit bc36bd1 into python:3.13 Nov 2, 2025
5 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@picnixz picnixz

Labels

topic-XML

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@hartwork @picnixz @ambv
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.