Skip to content

Commit 73fb894

Browse files
picnixzpicnixz
committed
pythongh-126037: fix UAF in xml.etree.ElementTree.Element.find* when concurrent mutations happen (python#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 1918799 commit 73fb894

File tree

3 files changed

+70
-49
lines changed

3 files changed

+70
-49
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: 29 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,29 +1249,28 @@ _elementtree_Element_find_impl(ElementObject *self, PyTypeObject *cls,
12491249
PyObject *path, PyObject *namespaces)
12501250
/*[clinic end generated code: output=18f77d393c9fef1b input=94df8a83f956acc6]*/
12511251
{
1252-
Py_ssize_t i;
12531252
elementtreestate *st = get_elementtree_state_by_cls(cls);
12541253

12551254
if (checkpath(path) || namespaces != Py_None) {
12561255
return PyObject_CallMethodObjArgs(
12571256
st->elementpath_obj, st->str_find, self, path, namespaces, NULL
1258-
);
1257+
);
12591258
}
12601259

1261-
if (!self->extra)
1262-
Py_RETURN_NONE;
1263-
1264-
for (i = 0; i < self->extra->length; i++) {
1265-
PyObject* item = self->extra->children[i];
1266-
int rc;
1260+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
1261+
PyObject *item = self->extra->children[i];
12671262
assert(Element_Check(st, item));
12681263
Py_INCREF(item);
1269-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1270-
if (rc > 0)
1264+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1265+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1266+
Py_DECREF(tag);
1267+
if (rc > 0) {
12711268
return item;
1269+
}
12721270
Py_DECREF(item);
1273-
if (rc < 0)
1271+
if (rc < 0) {
12741272
return NULL;
1273+
}
12751274
}
12761275

12771276
Py_RETURN_NONE;
@@ -1294,38 +1293,34 @@ _elementtree_Element_findtext_impl(ElementObject *self, PyTypeObject *cls,
12941293
PyObject *namespaces)
12951294
/*[clinic end generated code: output=6af7a2d96aac32cb input=32f252099f62a3d2]*/
12961295
{
1297-
Py_ssize_t i;
12981296
elementtreestate *st = get_elementtree_state_by_cls(cls);
12991297

13001298
if (checkpath(path) || namespaces != Py_None)
13011299
return PyObject_CallMethodObjArgs(
13021300
st->elementpath_obj, st->str_findtext,
13031301
self, path, default_value, namespaces, NULL
1304-
);
1305-
1306-
if (!self->extra) {
1307-
return Py_NewRef(default_value);
1308-
}
1302+
);
13091303

1310-
for (i = 0; i < self->extra->length; i++) {
1304+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
13111305
PyObject *item = self->extra->children[i];
1312-
int rc;
13131306
assert(Element_Check(st, item));
13141307
Py_INCREF(item);
1315-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1308+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1309+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1310+
Py_DECREF(tag);
13161311
if (rc > 0) {
1317-
PyObject* text = element_get_text((ElementObject*)item);
1312+
PyObject *text = element_get_text((ElementObject *)item);
1313+
Py_DECREF(item);
13181314
if (text == Py_None) {
1319-
Py_DECREF(item);
1320-
return PyUnicode_New(0, 0);
1315+
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
13211316
}
13221317
Py_XINCREF(text);
1323-
Py_DECREF(item);
13241318
return text;
13251319
}
13261320
Py_DECREF(item);
1327-
if (rc < 0)
1321+
if (rc < 0) {
13281322
return NULL;
1323+
}
13291324
}
13301325

13311326
return Py_NewRef(default_value);
@@ -1346,29 +1341,26 @@ _elementtree_Element_findall_impl(ElementObject *self, PyTypeObject *cls,
13461341
PyObject *path, PyObject *namespaces)
13471342
/*[clinic end generated code: output=65e39a1208f3b59e input=7aa0db45673fc9a5]*/
13481343
{
1349-
Py_ssize_t i;
1350-
PyObject* out;
13511344
elementtreestate *st = get_elementtree_state_by_cls(cls);
13521345

13531346
if (checkpath(path) || namespaces != Py_None) {
13541347
return PyObject_CallMethodObjArgs(
13551348
st->elementpath_obj, st->str_findall, self, path, namespaces, NULL
1356-
);
1349+
);
13571350
}
13581351

1359-
out = PyList_New(0);
1360-
if (!out)
1352+
PyObject *out = PyList_New(0);
1353+
if (out == NULL) {
13611354
return NULL;
1355+
}
13621356

1363-
if (!self->extra)
1364-
return out;
1365-
1366-
for (i = 0; i < self->extra->length; i++) {
1367-
PyObject* item = self->extra->children[i];
1368-
int rc;
1357+
for (Py_ssize_t i = 0; self->extra && i < self->extra->length; i++) {
1358+
PyObject *item = self->extra->children[i];
13691359
assert(Element_Check(st, item));
13701360
Py_INCREF(item);
1371-
rc = PyObject_RichCompareBool(((ElementObject*)item)->tag, path, Py_EQ);
1361+
PyObject *tag = Py_NewRef(((ElementObject *)item)->tag);
1362+
int rc = PyObject_RichCompareBool(tag, path, Py_EQ);
1363+
Py_DECREF(tag);
13721364
if (rc != 0 && (rc < 0 || PyList_Append(out, item) < 0)) {
13731365
Py_DECREF(item);
13741366
Py_DECREF(out);

0 commit comments

Comments
 (0)