Skip to content

Commit c57623c

Browse files
picnixzpicnixz
authored
gh-126037: fix UAF in xml.etree.ElementTree.Element.find* when concurrent mutations happen (#127964)
We fix a use-after-free in the `find`, `findtext` and `findall` methods of `xml.etree.ElementTree.Element` objects that can be triggered when the tag to find implements an `__eq__` method that mutates the element being queried.
1 parent 6aa88a2 commit c57623c

File tree

3 files changed

+69
-48
lines changed

3 files changed

+69
-48
lines changed

‎Lib/test/test_xml_etree.py‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2793,20 +2793,38 @@ def element_factory(x, y):
27932793
gc_collect()
27942794

27952795

2796-
class MutatingElementPath(str):
2796+
class MutationDeleteElementPath(str):
27972797
def __new__(cls, elem, *args):
27982798
self = str.__new__(cls, *args)
27992799
self.elem = elem
28002800
return self
2801+
28012802
def __eq__(self, o):
28022803
del self.elem[:]
28032804
return True
2804-
MutatingElementPath.__hash__ = str.__hash__
2805+
2806+
__hash__ = str.__hash__
2807+
2808+
2809+
class MutationClearElementPath(str):
2810+
def __new__(cls, elem, *args):
2811+
self = str.__new__(cls, *args)
2812+
self.elem = elem
2813+
return self
2814+
2815+
def __eq__(self, o):
2816+
self.elem.clear()
2817+
return True
2818+
2819+
__hash__ = str.__hash__
2820+
28052821

28062822
class BadElementPath(str):
28072823
def __eq__(self, o):
28082824
raise 1/0
2809-
BadElementPath.__hash__ = str.__hash__
2825+
2826+
__hash__ = str.__hash__
2827+
28102828

28112829
class BadElementPathTest(ElementTestCase, unittest.TestCase):
28122830
def setUp(self):
@@ -2821,9 +2839,11 @@ def tearDown(self):
28212839
super().tearDown()
28222840

28232841
def test_find_with_mutating(self):
2824-
e = ET.Element('foo')
2825-
e.extend([ET.Element('bar')])
2826-
e.find(MutatingElementPath(e, 'x'))
2842+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2843+
with self.subTest(cls):
2844+
e = ET.Element('foo')
2845+
e.extend([ET.Element('bar')])
2846+
e.find(cls(e, 'x'))
28272847

28282848
def test_find_with_error(self):
28292849
e = ET.Element('foo')
@@ -2834,9 +2854,11 @@ def test_find_with_error(self):
28342854
pass
28352855

28362856
def test_findtext_with_mutating(self):
2837-
e = ET.Element('foo')
2838-
e.extend([ET.Element('bar')])
2839-
e.findtext(MutatingElementPath(e, 'x'))
2857+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2858+
with self.subTest(cls):
2859+
e = ET.Element('foo')
2860+
e.extend([ET.Element('bar')])
2861+
e.findtext(cls(e, 'x'))
28402862

28412863
def test_findtext_with_error(self):
28422864
e = ET.Element('foo')
@@ -2861,9 +2883,11 @@ def test_findtext_with_none_text_attribute(self):
28612883
self.assertEqual(root_elem.findtext('./bar'), '')
28622884

28632885
def test_findall_with_mutating(self):
2864-
e = ET.Element('foo')
2865-
e.extend([ET.Element('bar')])
2866-
e.findall(MutatingElementPath(e, 'x'))
2886+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2887+
with self.subTest(cls):
2888+
e = ET.Element('foo')
2889+
e.extend([ET.Element('bar')])
2890+
e.findall(cls(e, 'x'))
28672891

28682892
def test_findall_with_error(self):
28692893
e = ET.Element('foo')

‎Misc/NEWS.d/next/Library/2024-12-15-15-07-22.gh-issue-126037.OyA7JP.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`xml.etree.ElementTree`: Fix a crash in :meth:`Element.find <xml.etree.ElementTree.Element.find>`,
2+
:meth:`Element.findtext <xml.etree.ElementTree.Element.findtext>` and
3+
:meth:`Element.findall <xml.etree.ElementTree.Element.findall>` when the tag
4+
to find implements an :meth:`~object.__eq__` method mutating the element
5+
being queried. Patch by Bénédikt Tran.

‎Modules/_elementtree.c‎

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,29 +1252,28 @@ _elementtree_Element_find_impl(ElementObject *self, PyTypeObject *cls,
12521252
PyObject *path, PyObject *namespaces)
12531253
/*[clinic end generated code: output=18f77d393c9fef1b input=94df8a83f956acc6]*/
12541254
{
1255-
Py_ssize_t i;
12561255
elementtreestate *st = get_elementtree_state_by_cls(cls);
12571256

12581257
if (checkpath(path) || namespaces != Py_None) {
12591258
return PyObject_CallMethodObjArgs(
12601259
st->elementpath_obj, st->str_find, self, path, namespaces, NULL
1261-
);
1260+
);
12621261
}
12631262

1264-
if (!self->extra)
1265-
Py_RETURN_NONE;
1266-
1267-
for (i = 0; i < self->extra->length; i++) {
1268-
PyObject* item = self->extra->children[i];
1269-
int rc;
1263+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
1264+
PyObject *item = self->extra->children[i];
12701265
assert(Element_Check(st, item));
12711266
Py_INCREF(item);
1272-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1273-
if (rc > 0)
1267+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1268+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1269+
Py_DECREF(tag);
1270+
if (rc > 0) {
12741271
return item;
1272+
}
12751273
Py_DECREF(item);
1276-
if (rc < 0)
1274+
if (rc < 0) {
12771275
return NULL;
1276+
}
12781277
}
12791278

12801279
Py_RETURN_NONE;
@@ -1297,38 +1296,34 @@ _elementtree_Element_findtext_impl(ElementObject *self, PyTypeObject *cls,
12971296
PyObject *namespaces)
12981297
/*[clinic end generated code: output=6af7a2d96aac32cb input=32f252099f62a3d2]*/
12991298
{
1300-
Py_ssize_t i;
13011299
elementtreestate *st = get_elementtree_state_by_cls(cls);
13021300

13031301
if (checkpath(path) || namespaces != Py_None)
13041302
return PyObject_CallMethodObjArgs(
13051303
st->elementpath_obj, st->str_findtext,
13061304
self, path, default_value, namespaces, NULL
1307-
);
1308-
1309-
if (!self->extra) {
1310-
return Py_NewRef(default_value);
1311-
}
1305+
);
13121306

1313-
for (i = 0; i < self->extra->length; i++) {
1307+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
13141308
PyObject *item = self->extra->children[i];
1315-
int rc;
13161309
assert(Element_Check(st, item));
13171310
Py_INCREF(item);
1318-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1311+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1312+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1313+
Py_DECREF(tag);
13191314
if (rc > 0) {
1320-
PyObject* text = element_get_text((ElementObject*)item);
1315+
PyObject *text = element_get_text((ElementObject *)item);
1316+
Py_DECREF(item);
13211317
if (text == Py_None) {
1322-
Py_DECREF(item);
13231318
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
13241319
}
13251320
Py_XINCREF(text);
1326-
Py_DECREF(item);
13271321
return text;
13281322
}
13291323
Py_DECREF(item);
1330-
if (rc < 0)
1324+
if (rc < 0) {
13311325
return NULL;
1326+
}
13321327
}
13331328

13341329
return Py_NewRef(default_value);
@@ -1349,29 +1344,26 @@ _elementtree_Element_findall_impl(ElementObject *self, PyTypeObject *cls,
13491344
PyObject *path, PyObject *namespaces)
13501345
/*[clinic end generated code: output=65e39a1208f3b59e input=7aa0db45673fc9a5]*/
13511346
{
1352-
Py_ssize_t i;
1353-
PyObject* out;
13541347
elementtreestate *st = get_elementtree_state_by_cls(cls);
13551348

13561349
if (checkpath(path) || namespaces != Py_None) {
13571350
return PyObject_CallMethodObjArgs(
13581351
st->elementpath_obj, st->str_findall, self, path, namespaces, NULL
1359-
);
1352+
);
13601353
}
13611354

1362-
out = PyList_New(0);
1363-
if (!out)
1355+
PyObject *out = PyList_New(0);
1356+
if (out == NULL) {
13641357
return NULL;
1358+
}
13651359

1366-
if (!self->extra)
1367-
return out;
1368-
1369-
for (i = 0; i < self->extra->length; i++) {
1370-
PyObject* item = self->extra->children[i];
1371-
int rc;
1360+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
1361+
PyObject *item = self->extra->children[i];
13721362
assert(Element_Check(st, item));
13731363
Py_INCREF(item);
1374-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1364+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1365+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1366+
Py_DECREF(tag);
13751367
if (rc != 0 && (rc < 0 || PyList_Append(out, item) < 0)) {
13761368
Py_DECREF(item);
13771369
Py_DECREF(out);

0 commit comments

Comments
 (0)