https://github.com/python/cpython/commit/b6b0e14b3d4aa9e9b89bef9a516177238883e1a7
commit: b6b0e14b3d4aa9e9b89bef9a516177238883e1a7
branch: main
author: Bénédikt Tran <[email protected]>
committer: picnixz <[email protected]>
date: 2025-12-29T18:30:51+01:00
summary:
gh-143200: fix UAFs in `Element.__{set,get}item__` when the element is
concurrently mutated (#143226)
files:
A Misc/NEWS.d/next/Library/2025-12-27-15-41-27.gh-issue-143200.2hEUAl.rst
M Lib/test/test_xml_etree.py
M Modules/_elementtree.c
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..22d3205e6ad314 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,26 @@ 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);
if (value == NULL) {
/* Delete slice */
- size_t cur;
- Py_ssize_t i;
-
+ if (self->extra == NULL) {
+ return 0;
+ }
+ slicelen = PySlice_AdjustIndices(self->extra->length, &start,
&stop,
+ step);
if (slicelen <= 0)
return 0;
@@ -1948,8 +1950,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 "
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: [email protected]
