Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 56 additions & 16 deletions Lib/test/test_xml_etree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
48 changes: 33 additions & 15 deletions Modules/_elementtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 "
Expand Down
Loading