Skip to content

gh-126033: Fix crash in _elementtree.c where evil tags/elements occurs #126079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

gougougougougou
Copy link

gh-126033: Fix crash in _elementtree.c where evil tags/elements occurs

Throws errors when evil tags/elements was detected.

This also fixes gh-126037.

@ghost
Copy link

ghost commented Oct 28, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Oct 28, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@picnixz
Copy link
Member

picnixz commented Oct 28, 2024

The way to fix this is not necessarily only by checking whether extra is NULL or not. You may also change the size of the list without clearing it and currently we don't check this. I think we should use RuntimeError instead as well because that's the kind of exceptions we usually raise when a list is mutated while iterating over it.

There are also other places where there is a crash (namely in other find* methods). I would appreciate we first discuss the approach for fixing it (one way to fix it is to add versioning but we need to discuss first).

Finally, I would have appreciated if you first asked whether you could open a PR or not since I was also working on it (and self-assigned the issue).

@gougougougougou
Copy link
Author

The way to fix this is not necessarily only by checking whether extra is NULL or not. You may also change the size of the list without clearing it and currently we don't check this. I think we should use RuntimeError instead as well because that's the kind of exceptions we usually raise when a list is mutated while iterating over it.

There are also other places where there is a crash (namely in other find* methods). I would appreciate we first discuss the approach for fixing it (one way to fix it is to add versioning but we need to discuss first).

Finally, I would have appreciated if you first asked whether you could open a PR or not since I was also working on it (and self-assigned the issue).

Sorry about that, I didn’t saw the issue carefully about the self-assigning tag. I’ve understand your solution about this issue, For my solution, just adding the error throwing for just few functions, doesn't really fit for this issue. so I’ll close this PR soon.

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.

UAF: xml.etree.ElementTree.Element.find* when concurrent mutations happen
2 participants