GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||
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
|
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
(Segfaults could be because you didn't |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The address sanitizer says that there is a heap-UAF. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…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)
|
@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 I'm attaching my three diffs if you would like to see my cross-diff comparison with your own eyes:
Looking from a different angle next… |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@picnixz I found that… Two (or arguably four) tests are failing:
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 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Anyway, we can correctly assume that we have a UAF. Let's create a separate issue (do you want to?) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
@picnixz good with me, I can revert the workaround later 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
To have a good synchronization, we'll also delay 3.10 to 3.13 backports for their next release cycle (see #139359 (comment)). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I set DO-NOT-MERGE to avoid confusion. Unset that when you think we should be releasing this. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The commit message will be I'll rerun the CI |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@picnixz done for 3.{10..13} just now 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@picnixz understood, yes 👍
Do you want me to summarize the difference between these four backports?
Thank you! 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.