Skip to content

Commit f1689b6

Browse files
picnixzpicnixz
authored
[3.12] pythongh-126037: fix UAF in xml.etree.ElementTree.Element.find* when concurrent mutations happen (python#127964) (python#131932)
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. (cherry picked from commit c57623c)
1 parent 525eddf commit f1689b6

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
@@ -2713,20 +2713,38 @@ def element_factory(x, y):
27132713
gc_collect()
27142714

27152715

2716-
class MutatingElementPath(str):
2716+
class MutationDeleteElementPath(str):
27172717
def __new__(cls, elem, *args):
27182718
self = str.__new__(cls, *args)
27192719
self.elem = elem
27202720
return self
2721+
27212722
def __eq__(self, o):
27222723
del self.elem[:]
27232724
return True
2724-
MutatingElementPath.__hash__ = str.__hash__
2725+
2726+
__hash__ = str.__hash__
2727+
2728+
2729+
class MutationClearElementPath(str):
2730+
def __new__(cls, elem, *args):
2731+
self = str.__new__(cls, *args)
2732+
self.elem = elem
2733+
return self
2734+
2735+
def __eq__(self, o):
2736+
self.elem.clear()
2737+
return True
2738+
2739+
__hash__ = str.__hash__
2740+
27252741

27262742
class BadElementPath(str):
27272743
def __eq__(self, o):
27282744
raise 1/0
2729-
BadElementPath.__hash__ = str.__hash__
2745+
2746+
__hash__ = str.__hash__
2747+
27302748

27312749
class BadElementPathTest(ElementTestCase, unittest.TestCase):
27322750
def setUp(self):
@@ -2741,9 +2759,11 @@ def tearDown(self):
27412759
super().tearDown()
27422760

27432761
def test_find_with_mutating(self):
2744-
e = ET.Element('foo')
2745-
e.extend([ET.Element('bar')])
2746-
e.find(MutatingElementPath(e, 'x'))
2762+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2763+
with self.subTest(cls):
2764+
e = ET.Element('foo')
2765+
e.extend([ET.Element('bar')])
2766+
e.find(cls(e, 'x'))
27472767

27482768
def test_find_with_error(self):
27492769
e = ET.Element('foo')
@@ -2754,9 +2774,11 @@ def test_find_with_error(self):
27542774
pass
27552775

27562776
def test_findtext_with_mutating(self):
2757-
e = ET.Element('foo')
2758-
e.extend([ET.Element('bar')])
2759-
e.findtext(MutatingElementPath(e, 'x'))
2777+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2778+
with self.subTest(cls):
2779+
e = ET.Element('foo')
2780+
e.extend([ET.Element('bar')])
2781+
e.findtext(cls(e, 'x'))
27602782

27612783
def test_findtext_with_error(self):
27622784
e = ET.Element('foo')
@@ -2781,9 +2803,11 @@ def test_findtext_with_none_text_attribute(self):
27812803
self.assertEqual(root_elem.findtext('./bar'), '')
27822804

27832805
def test_findall_with_mutating(self):
2784-
e = ET.Element('foo')
2785-
e.extend([ET.Element('bar')])
2786-
e.findall(MutatingElementPath(e, 'x'))
2806+
for cls in [MutationDeleteElementPath, MutationClearElementPath]:
2807+
with self.subTest(cls):
2808+
e = ET.Element('foo')
2809+
e.extend([ET.Element('bar')])
2810+
e.findall(cls(e, 'x'))
27872811

27882812
def test_findall_with_error(self):
27892813
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
@@ -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);
13201315
return PyUnicode_New(0, 0);
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)