Skip to content

gh-148735: Fix a UAF in Element.findtext() #148738

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.

Sign up for GitHub

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

Open
StanFromIreland wants to merge 2 commits into
base: main
Choose a base branch
from
+23 −13
Open

gh-148735: Fix a UAF in Element.findtext() #148738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae55e9c
Fix a UAF in
StanFromIreland Apr 18, 2026
a8d4139
Return a new ref in `element_get_{tail,text}` instead
StanFromIreland Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Lib/test/test_xml_etree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3258,6 +3258,16 @@ def test_findtext_with_mutating(self):
e.extend([ET.Element('bar')])
e.findtext(cls(e, 'x'))

def test_findtext_with_mutating_non_none_text(self):
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
with self.subTest(cls):
e = ET.Element('foo')
child = ET.Element('bar')
child.text = str(object())
e.append(child)
del child
repr(e.findtext(cls(e, 'x')))

def test_findtext_with_error(self):
e = ET.Element('foo')
e.extend([ET.Element('bar')])
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2026-04-18-21-39-15.gh-issue-148735.siw6DG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:mod:`xml.etree.ElementTree`: Fix a use-after-free in
:meth:`Element.findtext <xml.etree.ElementTree.Element.findtext>` when the
tag to find implements an :meth:`~object.__eq__` method that drops every
Copy link
Copy Markdown
Member

@picnixz picnixz Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just mention that this happens when there are mutations. Using eq is one possibility (probably the only one really easy to trigger though)

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This describes how to reproduce the issue. But this is not what happens in most user code. Users rarely implement __eq__ which intentionally mutates the element. More realistic scenario -- it is mutated concurrently with running findtext(). This usually happens with threads, but can also happen with garbage collector or indirectly in __eq__. Anyway, the cause is a concurrent mutation.

other reference to a matching element.
22 changes: 9 additions & 13 deletions Modules/_elementtree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ element_get_attrib(ElementObject* self)
LOCAL(PyObject*)
element_get_text(ElementObject* self)
{
/* return borrowed reference to text attribute */
/* return new reference to text attribute */

PyObject *res = self->text;

Expand All @@ -587,13 +587,13 @@ element_get_text(ElementObject* self)
}
}

return res;
return Py_NewRef(res);
}

LOCAL(PyObject*)
element_get_tail(ElementObject* self)
{
/* return borrowed reference to text attribute */
/* return new reference to tail attribute */

PyObject *res = self->tail;

Expand All @@ -608,7 +608,7 @@ element_get_tail(ElementObject* self)
}
}

return res;
return Py_NewRef(res);
}

static PyObject*
Expand Down Expand Up @@ -1350,9 +1350,9 @@ _elementtree_Element_findtext_impl(ElementObject *self, PyTypeObject *cls,
PyObject *text = element_get_text((ElementObject *)item);
Py_DECREF(item);
if (text == Py_None) {
Py_DECREF(text);
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
}
Py_XINCREF(text);
return text;
}
Py_DECREF(item);
Expand Down Expand Up @@ -2055,16 +2055,14 @@ static PyObject*
element_text_getter(PyObject *op, void *closure)
{
ElementObject *self = _Element_CAST(op);
PyObject *res = element_get_text(self);
return Py_XNewRef(res);
return element_get_text(self);
}

static PyObject*
element_tail_getter(PyObject *op, void *closure)
{
ElementObject *self = _Element_CAST(op);
PyObject *res = element_get_tail(self);
return Py_XNewRef(res);
return element_get_tail(self);
}

static PyObject*
Expand Down Expand Up @@ -2307,16 +2305,14 @@ elementiter_next(PyObject *op)
continue;

gettext:
Py_DECREF(elem);
if (!text) {
Py_DECREF(elem);
return NULL;
}
if (text == Py_None) {
Py_DECREF(elem);
Py_DECREF(text);
}
else {
Py_INCREF(text);
Py_DECREF(elem);
rc = PyObject_IsTrue(text);
if (rc > 0)
return text;
Expand Down
Loading