Skip to content

Commit 93cb42f

Browse files
[3.13] gh-143200: fix UAFs in Element.__{set,get}item__ when the element is concurrently mutated (GH-143226) (#143274)
gh-143200: fix UAFs in `Element.__{set,get}item__` when the element is concurrently mutated (GH-143226) (cherry picked from commit b6b0e14) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent 8cfe1ab commit 93cb42f

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
@@ -2964,32 +2964,72 @@ def __del__(self):
29642964
elem = b.close()
29652965
self.assertEqual(elem[0].tail, 'ABCDEFGHIJKL')
29662966

2967-
def test_subscr(self):
2968-
# Issue #27863
2967+
def test_subscr_with_clear(self):
2968+
# See https://github.com/python/cpython/issues/143200.
2969+
self.do_test_subscr_with_mutating_slice(use_clear_method=True)
2970+
2971+
def test_subscr_with_delete(self):
2972+
# See https://github.com/python/cpython/issues/72050.
2973+
self.do_test_subscr_with_mutating_slice(use_clear_method=False)
2974+
2975+
def do_test_subscr_with_mutating_slice(self, *, use_clear_method):
29692976
class X:
2977+
def __init__(self, i=0):
2978+
self.i = i
29702979
def __index__(self):
2971-
del e[:]
2972-
return 1
2980+
if use_clear_method:
2981+
e.clear()
2982+
else:
2983+
del e[:]
2984+
return self.i
29732985

2974-
e = ET.Element('elem')
2975-
e.append(ET.Element('child'))
2976-
e[:X()] # shouldn't crash
2986+
for s in self.get_mutating_slices(X, 10):
2987+
with self.subTest(s):
2988+
e = ET.Element('elem')
2989+
e.extend([ET.Element(f'c{i}') for i in range(10)])
2990+
e[s] # shouldn't crash
29772991

2978-
e.append(ET.Element('child'))
2979-
e[0:10:X()] # shouldn't crash
2992+
def test_ass_subscr_with_mutating_slice(self):
2993+
# See https://github.com/python/cpython/issues/72050
2994+
# and https://github.com/python/cpython/issues/143200.
29802995

2981-
def test_ass_subscr(self):
2982-
# Issue #27863
29832996
class X:
2997+
def __init__(self, i=0):
2998+
self.i = i
29842999
def __index__(self):
29853000
e[:] = []
2986-
return 1
3001+
return self.i
3002+
3003+
for s in self.get_mutating_slices(X, 10):
3004+
with self.subTest(s):
3005+
e = ET.Element('elem')
3006+
e.extend([ET.Element(f'c{i}') for i in range(10)])
3007+
e[s] = [] # shouldn't crash
3008+
3009+
def get_mutating_slices(self, index_class, n_children):
3010+
self.assertGreaterEqual(n_children, 10)
3011+
return [
3012+
slice(index_class(), None, None),
3013+
slice(index_class(2), None, None),
3014+
slice(None, index_class(), None),
3015+
slice(None, index_class(2), None),
3016+
slice(0, 2, index_class(1)),
3017+
slice(0, 2, index_class(2)),
3018+
slice(0, n_children, index_class(1)),
3019+
slice(0, n_children, index_class(2)),
3020+
slice(0, 2 * n_children, index_class(1)),
3021+
slice(0, 2 * n_children, index_class(2)),
3022+
]
29873023

2988-
e = ET.Element('elem')
2989-
for _ in range(10):
2990-
e.insert(0, ET.Element('child'))
3024+
def test_ass_subscr_with_mutating_iterable_value(self):
3025+
class V:
3026+
def __iter__(self):
3027+
e.clear()
3028+
return iter([ET.Element('a'), ET.Element('b')])
29913029

2992-
e[0:10:X()] = [] # shouldn't crash
3030+
e = ET.Element('elem')
3031+
e.extend([ET.Element(f'c{i}') for i in range(10)])
3032+
e[:] = V()
29933033

29943034
def test_treebuilder_start(self):
29953035
# 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
@@ -1803,16 +1803,20 @@ element_subscr(PyObject* self_, PyObject* item)
18031803
return element_getitem(self_, i);
18041804
}
18051805
else if (PySlice_Check(item)) {
1806+
// Note: 'slicelen' is computed once we are sure that 'self->extra'
1807+
// cannot be mutated by user-defined code.
1808+
// See https://github.com/python/cpython/issues/143200.
18061809
Py_ssize_t start, stop, step, slicelen, i;
18071810
size_t cur;
18081811
PyObject* list;
18091812

1810-
if (!self->extra)
1811-
return PyList_New(0);
1812-
18131813
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
18141814
return NULL;
18151815
}
1816+
1817+
if (self->extra == NULL) {
1818+
return PyList_New(0);
1819+
}
18161820
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
18171821
step);
18181822

@@ -1855,28 +1859,26 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
18551859
return element_setitem(self_, i, value);
18561860
}
18571861
else if (PySlice_Check(item)) {
1862+
// Note: 'slicelen' is computed once we are sure that 'self->extra'
1863+
// cannot be mutated by user-defined code.
1864+
// See https://github.com/python/cpython/issues/143200.
18581865
Py_ssize_t start, stop, step, slicelen, newlen, i;
18591866
size_t cur;
18601867

18611868
PyObject* recycle = NULL;
18621869
PyObject* seq;
18631870

1864-
if (!self->extra) {
1865-
if (create_extra(self, NULL) < 0)
1866-
return -1;
1867-
}
1868-
18691871
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
18701872
return -1;
18711873
}
1872-
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
1873-
step);
18741874

18751875
if (value == NULL) {
18761876
/* Delete slice */
1877-
size_t cur;
1878-
Py_ssize_t i;
1879-
1877+
if (self->extra == NULL) {
1878+
return 0;
1879+
}
1880+
slicelen = PySlice_AdjustIndices(self->extra->length, &start, &stop,
1881+
step);
18801882
if (slicelen <= 0)
18811883
return 0;
18821884

@@ -1945,8 +1947,16 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
19451947
}
19461948
newlen = PySequence_Fast_GET_SIZE(seq);
19471949

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

0 commit comments

Comments
 (0)