Skip to content

Commit b6b0e14

Browse files
authored
gh-143200: fix UAFs in Element.__{set,get}item__ when the element is concurrently mutated (#143226)
1 parent 6cb245d commit b6b0e14

File tree

3 files changed

+85
-31
lines changed

3 files changed

+85
-31
lines changed

Lib/test/test_xml_etree.py

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3010,32 +3010,72 @@ def __del__(self):
30103010
elem = b.close()
30113011
self.assertEqual(elem[0].tail, 'ABCDEFGHIJKL')
30123012

3013-
def test_subscr(self):
3014-
# Issue #27863
3013+
def test_subscr_with_clear(self):
3014+
# See https://github.com/python/cpython/issues/143200.
3015+
self.do_test_subscr_with_mutating_slice(use_clear_method=True)
3016+
3017+
def test_subscr_with_delete(self):
3018+
# See https://github.com/python/cpython/issues/72050.
3019+
self.do_test_subscr_with_mutating_slice(use_clear_method=False)
3020+
3021+
def do_test_subscr_with_mutating_slice(self, *, use_clear_method):
30153022
class X:
3023+
def __init__(self, i=0):
3024+
self.i = i
30163025
def __index__(self):
3017-
del e[:]
3018-
return 1
3026+
if use_clear_method:
3027+
e.clear()
3028+
else:
3029+
del e[:]
3030+
return self.i
30193031

3020-
e = ET.Element('elem')
3021-
e.append(ET.Element('child'))
3022-
e[:X()] # shouldn't crash
3032+
for s in self.get_mutating_slices(X, 10):
3033+
with self.subTest(s):
3034+
e = ET.Element('elem')
3035+
e.extend([ET.Element(f'c{i}') for i in range(10)])
3036+
e[s] # shouldn't crash
30233037

3024-
e.append(ET.Element('child'))
3025-
e[0:10:X()] # shouldn't crash
3038+
def test_ass_subscr_with_mutating_slice(self):
3039+
# See https://github.com/python/cpython/issues/72050
3040+
# and https://github.com/python/cpython/issues/143200.
30263041

3027-
def test_ass_subscr(self):
3028-
# Issue #27863
30293042
class X:
3043+
def __init__(self, i=0):
3044+
self.i = i
30303045
def __index__(self):
30313046
e[:] = []
3032-
return 1
3047+
return self.i
3048+
3049+
for s in self.get_mutating_slices(X, 10):
3050+
with self.subTest(s):
3051+
e = ET.Element('elem')
3052+
e.extend([ET.Element(f'c{i}') for i in range(10)])
3053+
e[s] = [] # shouldn't crash
3054+
3055+
def get_mutating_slices(self, index_class, n_children):
3056+
self.assertGreaterEqual(n_children, 10)
3057+
return [
3058+
slice(index_class(), None, None),
3059+
slice(index_class(2), None, None),
3060+
slice(None, index_class(), None),
3061+
slice(None, index_class(2), None),
3062+
slice(0, 2, index_class(1)),
3063+
slice(0, 2, index_class(2)),
3064+
slice(0, n_children, index_class(1)),
3065+
slice(0, n_children, index_class(2)),
3066+
slice(0, 2 * n_children, index_class(1)),
3067+
slice(0, 2 * n_children, index_class(2)),
3068+
]
30333069

3034-
e = ET.Element('elem')
3035-
for _ in range(10):
3036-
e.insert(0, ET.Element('child'))
3070+
def test_ass_subscr_with_mutating_iterable_value(self):
3071+
class V:
3072+
def __iter__(self):
3073+
e.clear()
3074+
return iter([ET.Element('a'), ET.Element('b')])
30373075

3038-
e[0:10:X()] = [] # shouldn't crash
3076+
e = ET.Element('elem')
3077+
e.extend([ET.Element(f'c{i}') for i in range(10)])
3078+
e[:] = V()
30393079

30403080
def test_treebuilder_start(self):
30413081
# Issue #27863
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
:mod:`xml.etree.ElementTree`: fix use-after-free crashes in
2+
:meth:`~object.__getitem__` and :meth:`~object.__setitem__` methods of
3+
:class:`~xml.etree.ElementTree.Element` when the element is concurrently
4+
mutated. Patch by Bénédikt Tran.

Modules/_elementtree.c

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,16 +1806,20 @@ element_subscr(PyObject *op, PyObject *item)
18061806
return element_getitem(op, i);
18071807
}
18081808
else if (PySlice_Check(item)) {
1809+
// Note: 'slicelen' is computed once we are sure that 'self->extra'
1810+
// cannot be mutated by user-defined code.
1811+
// See https://github.com/python/cpython/issues/143200.
18091812
Py_ssize_t start, stop, step, slicelen, i;
18101813
size_t cur;
18111814
PyObject* list;
18121815

1813-
if (!self->extra)
1814-
return PyList_New(0);
1815-
18161816
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
18171817
return NULL;
18181818
}
1819+
1820+
if (self->extra == NULL) {
1821+
return PyList_New(0);
1822+
}
18191823
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
18201824
step);
18211825

@@ -1858,28 +1862,26 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
18581862
return element_setitem(op, i, value);
18591863
}
18601864
else if (PySlice_Check(item)) {
1865+
// Note: 'slicelen' is computed once we are sure that 'self->extra'
1866+
// cannot be mutated by user-defined code.
1867+
// See https://github.com/python/cpython/issues/143200.
18611868
Py_ssize_t start, stop, step, slicelen, newlen, i;
18621869
size_t cur;
18631870

18641871
PyObject* recycle = NULL;
18651872
PyObject* seq;
18661873

1867-
if (!self->extra) {
1868-
if (create_extra(self, NULL) < 0)
1869-
return -1;
1870-
}
1871-
18721874
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
18731875
return -1;
18741876
}
1875-
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
1876-
step);
18771877

18781878
if (value == NULL) {
18791879
/* Delete slice */
1880-
size_t cur;
1881-
Py_ssize_t i;
1882-
1880+
if (self->extra == NULL) {
1881+
return 0;
1882+
}
1883+
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
1884+
step);
18831885
if (slicelen <= 0)
18841886
return 0;
18851887

@@ -1948,8 +1950,16 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
19481950
}
19491951
newlen = PySequence_Fast_GET_SIZE(seq);
19501952

1951-
if (step != 1 && newlen != slicelen)
1952-
{
1953+
if (self->extra == NULL) {
1954+
if (create_extra(self, NULL) < 0) {
1955+
Py_DECREF(seq);
1956+
return -1;
1957+
}
1958+
}
1959+
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
1960+
step);
1961+
1962+
if (step != 1 && newlen != slicelen) {
19531963
Py_DECREF(seq);
19541964
PyErr_Format(PyExc_ValueError,
19551965
"attempt to assign sequence of size %zd "

0 commit comments

Comments
 (0)