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 — UAF: `xml.etree.ElementTree.Element.remove` when concurrent mutations happen · Issue #126033 · python/cpython

UAF: xml.etree.ElementTree.Element.remove when concurrent mutations happen #126033

New issue
Copy link
New issue
Copy link
Closed
Closed
UAF: xml.etree.ElementTree.Element.remove when concurrent mutations happen#126033
Copy link
Assignees
picnixz
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesextension-modulesC modules in the Modules dirC modules in the Modules dirtopic-XMLtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

Description

@picnixz
picnixz
opened on Oct 27, 2024
Issue body actions

Crash report

What happened?

A UAF in Element.remove was fixed in #68279 but one can mutate the child's list during .remove and cause an OOB crash:

import xml.etree.ElementTree as ET

class EvilElement(ET.Element):
    def __eq__(self, other):
        base.clear()
        return False

base = ET.Element('a')
base.append(EvilElement('a'))
base.append(EvilElement('a'))
base.remove(ET.Element('b'))

Attacked code:

cpython/Modules/_elementtree.c

Lines 1648 to 1656 in dc76a4a

for (i = 0; i < self->extra->length; i++) {
if (self->extra->children[i] == subelement)
break;
rc = PyObject_RichCompareBool(self->extra->children[i], subelement, Py_EQ);
if (rc > 0)
break;
if (rc < 0)
return NULL;
}

I think we need to introduce some state integer to check that there is no evil mutation (similar to what's being done for OrderedDict).

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Output from running 'python -VV' on the command line:

No response

Linked PRs

  • gh-126033: Fix crash in _elementtree.c where evil tags/elements occurs #126079
  • gh-126033: fix a crash in xml.etree.ElementTree.Element.remove when concurrent mutations happen #126124
  • [3.13] gh-126033: fix UAF in xml.etree.ElementTree.Element.remove when concurrent mutations happen (GH-126124) #131929
  • [3.12] gh-126033: fix UAF in xml.etree.ElementTree.Element.remove when concurrent mutations happen (GH-126124) #131930

Activity

  1. picnixz
    added
    extension-modulesC modules in the Modules dirC modules in the Modules dir
    topic-XML
    type-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump
    on Oct 27, 2024
  2. ZeroIntensity commented on Oct 27, 2024

    @ZeroIntensity
    ZeroIntensity
    on Oct 27, 2024
    Member
    More actions

    Hmm, there seem to be a lot of issues with PyObject_RichCompareBool and custom __eq__'s, right? It might be worth trying to fix things more globally there rather than on a case-by-case basis like this.

  3. picnixz commented on Oct 27, 2024

    @picnixz
    picnixz
    on Oct 27, 2024
    MemberAuthor
    More actions

    I've roughly skimmed through the other occurrences and couldn't find UAFs or bad OOB. And I'd prefer fixing them one by one because the patch is not always the same.

    Reacted by Peter Bierma
  4. picnixz
    self-assigned this
    on Oct 27, 2024
  5. picnixz commented on Oct 27, 2024

    @picnixz
    picnixz
    on Oct 27, 2024 · Hidden as resolved
    Author
    show commentMore actions
  6. picnixz
    added
    3.12only security fixesonly security fixes
    3.13only security fixesonly security fixes
    3.14bugs and security fixesbugs and security fixes
    on Oct 27, 2024
  7. ZeroIntensity commented on Oct 27, 2024

    @ZeroIntensity
    ZeroIntensity
    on Oct 27, 2024
    Member
    More actions

    My main concern here is that while we can fix this as much as we want for the core, this looks like it's common enough that downstream is having this problem too.

  8. sobolevn commented on Oct 28, 2024

    @sobolevn
    sobolevn
    on Oct 28, 2024
    Member
    More actions

    Basically, we need to protect base from mutation during the iteration over it. An extra field maybe?

  9. picnixz commented on Oct 28, 2024

    @picnixz
    picnixz
    on Oct 28, 2024
    MemberAuthor
    More actions

    See #126041 for that (the issue and PR may have passed under your radar :))

  10. picnixz
    changed the title [-]Crash: `xml.etree.ElementTree.Element.remove` with an evil `Element.__eq__`[/-] [+]Crash: `xml.etree.ElementTree.Element.remove` when concurrent mutations happen[/+] on Dec 17, 2024
  11. picnixz
    changed the title [-]Crash: `xml.etree.ElementTree.Element.remove` when concurrent mutations happen[/-] [+]UAF: `xml.etree.ElementTree.Element.remove` when concurrent mutations happen[/+] on Mar 31, 2025
  12. picnixz
    added a commit that references this issue on Mar 31, 2025

    gh-126033: fix UAF in `xml.etree.ElementTree.Element.remove` when con…

    bab1398
  13. miss-islington
    added 2 commits that reference this issue on Mar 31, 2025

    pythongh-126033: fix UAF in `xml.etree.ElementTree.Element.remove` wh…

    1c6193c

    pythongh-126033: fix UAF in `xml.etree.ElementTree.Element.remove` wh…

    e1aa52c
  14. picnixz
    added 2 commits that reference this issue on Mar 31, 2025

    [3.13] gh-126033: fix UAF in `xml.etree.ElementTree.Element.remove` w…

    b41c8cc

    [3.12] gh-126033: fix UAF in `xml.etree.ElementTree.Element.remove` w…

    5d4e891
  15. picnixz
    closed this as completedon Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

  • @picnixz
    picnixz

Labels

3.12only security fixesonly security fixes3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesextension-modulesC modules in the Modules dirC modules in the Modules dirtopic-XMLtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    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.