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.14] gh-90949: add Expat API to prevent XML deadly allocations (CVE-2025-59375) (GH-139234) by picnixz · Pull Request #139359 · python/cpython Conversation
|
|
I'll wait until gh-139366 is merged to ease future backports |
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.
@picnixz if I'm not mistaken, it's both merged and cherry-picked to this PR (as the third commit) by now. What would be the next steps for this pull request? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
A core developer needs to review it, and the release manager needs to approve it because we're in RC3 phase. I really want at least two core devs to check this PR to be sure I didn't mess the backports. I didn't backport the docs changes that will be added in the next one (the one for billion laughs). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@picnixz so the plan is to first get review from Gregory and only then Hugo, I see 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I would be happy if you could also review it as you're an expert here as well :') |
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 a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I would be happy if you could also review it as you're an expert here as well :')
@picnixz done, looks good 👍
My method of review has been comparing (1) the diff of #139234 against main with (2) the diff in here. What I found was no difference but:
Doc/whatsnew/3.15.rst on main side but not 3.14 (as expected)@permit_long_* on main side but not 3.14 (as expected)The tools I used for this were git rebase -i, git diff and Meld.
So if #139234 was alright (which I believe) then very likely this is, too 👍
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thank you for the review. I'll let Gregory have a look and then Hugo can hit merge I think (only he can merge during RC phase). For the other branches, I would suggest you continue your branch for 3.13 with the latest stuff (or try to directly cherry-pick this specific PR with a new one). I think we should be able to backport this PR to 3.13 directly without too many conflicts (I hope). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
FWIW when to merge this is up to the release manager (hugovk). As we're treating it as a security mitigation feature similar to past such things and adding it to older releases as well, that suggests letting this wait for 3.14.1 is fine. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Note for the RM: we have a UAF in all branches, independently of Expat version: See #139400. So I would prefer that this one is not backported until we fix it. On 3.14 we don't see the crash, but on 3.13 (and probably on older branches), we will also see it. I believe we are able not to crash by chance due to some magic in incremental GC. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…and a WIP candidate fix at #139403.
@picnixz In my tests on another notebook I saw two other branches affected directly and once we add |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We will merge this one in 3.14.1 (3.14 is now ongoing its release process). To have a good synchronization, we'll also delay 3.10 to 3.13 backports for their next release cycle. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'm going to work on merging the deadly allocation PR now. Sorry for the delay but I got very busy with my new work & IRL |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This reverts commit 41b0aa2.
|
Ok, so here's the plan:
@hartwork Can you check your existing PRs please? I think it'd be fine to actually merge the 3.10-3.14 ones and their backports as we have roughly 2 months until the next release. |
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 yes please, a backport is not the place to fix up on the original as far as I am concerned.
I believe I got them all into mergable shape and they just need a re-trigger of CI — the read-the-docs one is flaky — to be green and ready. I am missing something? Anything in particular that you'd want me to check or re-check for?
Cool! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@picnixz thank you! 🙏 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@picnixz they all end in commit "Drop workaround for 139400 as no-longer-needed" and their commits tell the same story. I'm happy to fix anything that needs fixing, but I believe they're all ready and have been. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Perfect, I'm going to merge them all and then we'll move on to adding the other API (with the docs amendments) |
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.