diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py index 0178ed02b35be1..7aa949b2819172 100644 --- a/Lib/test/test_xml_etree.py +++ b/Lib/test/test_xml_etree.py @@ -3010,32 +3010,72 @@ def __del__(self): elem = b.close() self.assertEqual(elem[0].tail, 'ABCDEFGHIJKL') - def test_subscr(self): - # Issue #27863 + def test_subscr_with_clear(self): + # See https://github.com/python/cpython/issues/143200. + self.do_test_subscr_with_mutating_slice(use_clear_method=True) + + def test_subscr_with_delete(self): + # See https://github.com/python/cpython/issues/72050. + self.do_test_subscr_with_mutating_slice(use_clear_method=False) + + def do_test_subscr_with_mutating_slice(self, *, use_clear_method): class X: + def __init__(self, i=0): + self.i = i def __index__(self): - del e[:] - return 1 + if use_clear_method: + e.clear() + else: + del e[:] + return self.i - e = ET.Element('elem') - e.append(ET.Element('child')) - e[:X()] # shouldn't crash + for s in self.get_mutating_slices(X, 10): + with self.subTest(s): + e = ET.Element('elem') + e.extend([ET.Element(f'c{i}') for i in range(10)]) + e[s] # shouldn't crash - e.append(ET.Element('child')) - e[0:10:X()] # shouldn't crash + def test_ass_subscr_with_mutating_slice(self): + # See https://github.com/python/cpython/issues/72050 + # and https://github.com/python/cpython/issues/143200. - def test_ass_subscr(self): - # Issue #27863 class X: + def __init__(self, i=0): + self.i = i def __index__(self): e[:] = [] - return 1 + return self.i + + for s in self.get_mutating_slices(X, 10): + with self.subTest(s): + e = ET.Element('elem') + e.extend([ET.Element(f'c{i}') for i in range(10)]) + e[s] = [] # shouldn't crash + + def get_mutating_slices(self, index_class, n_children): + self.assertGreaterEqual(n_children, 10) + return [ + slice(index_class(), None, None), + slice(index_class(2), None, None), + slice(None, index_class(), None), + slice(None, index_class(2), None), + slice(0, 2, index_class(1)), + slice(0, 2, index_class(2)), + slice(0, n_children, index_class(1)), + slice(0, n_children, index_class(2)), + slice(0, 2 * n_children, index_class(1)), + slice(0, 2 * n_children, index_class(2)), + ] - e = ET.Element('elem') - for _ in range(10): - e.insert(0, ET.Element('child')) + def test_ass_subscr_with_mutating_iterable_value(self): + class V: + def __iter__(self): + e.clear() + return iter([ET.Element('a'), ET.Element('b')]) - e[0:10:X()] = [] # shouldn't crash + e = ET.Element('elem') + e.extend([ET.Element(f'c{i}') for i in range(10)]) + e[:] = V() def test_treebuilder_start(self): # Issue #27863 diff --git a/Misc/NEWS.d/next/Library/2025-12-27-15-41-27.gh-issue-143200.2hEUAl.rst b/Misc/NEWS.d/next/Library/2025-12-27-15-41-27.gh-issue-143200.2hEUAl.rst new file mode 100644 index 00000000000000..8b24decf098745 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-27-15-41-27.gh-issue-143200.2hEUAl.rst @@ -0,0 +1,4 @@ +:mod:`xml.etree.ElementTree`: fix use-after-free crashes in +:meth:`~object.__getitem__` and :meth:`~object.__setitem__` methods of +:class:`~xml.etree.ElementTree.Element` when the element is concurrently +mutated. Patch by Bénédikt Tran. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 3173b52afb31b6..d93b23d18143a4 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -1806,16 +1806,20 @@ element_subscr(PyObject *op, PyObject *item) return element_getitem(op, i); } else if (PySlice_Check(item)) { + // Note: 'slicelen' is computed once we are sure that 'self->extra' + // cannot be mutated by user-defined code. + // See https://github.com/python/cpython/issues/143200. Py_ssize_t start, stop, step, slicelen, i; size_t cur; PyObject* list; - if (!self->extra) - return PyList_New(0); - if (PySlice_Unpack(item, &start, &stop, &step) < 0) { return NULL; } + + if (self->extra == NULL) { + return PyList_New(0); + } slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop, step); @@ -1858,28 +1862,34 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value) return element_setitem(op, i, value); } else if (PySlice_Check(item)) { + // Note: 'slicelen' is computed once we are sure that 'self->extra' + // cannot be mutated by user-defined code. + // See https://github.com/python/cpython/issues/143200. Py_ssize_t start, stop, step, slicelen, newlen, i; size_t cur; PyObject* recycle = NULL; PyObject* seq; - if (!self->extra) { - if (create_extra(self, NULL) < 0) - return -1; - } - if (PySlice_Unpack(item, &start, &stop, &step) < 0) { return -1; } - slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop, - step); + + // Since PySlice_Unpack() may clear 'self->extra', we need + // to ensure that it exists now (using a slice for assignment + // should not raise IndexError). + // + // See https://github.com/python/cpython/issues/143200. + if (self->extra == NULL) { + if (create_extra(self, NULL) < 0) { + return -1; + } + } if (value == NULL) { /* Delete slice */ - size_t cur; - Py_ssize_t i; - + slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop, + step); if (slicelen <= 0) return 0; @@ -1948,8 +1958,16 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value) } newlen = PySequence_Fast_GET_SIZE(seq); - if (step != 1 && newlen != slicelen) - { + if (self->extra == NULL) { + if (create_extra(self, NULL) < 0) { + Py_DECREF(seq); + return -1; + } + } + slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop, + step); + + if (step != 1 && newlen != slicelen) { Py_DECREF(seq); PyErr_Format(PyExc_ValueError, "attempt to assign sequence of size %zd "