bpo-27863: Fixed multiple crashes in ElementTree. (#765) (#903)

Lib/test/test_xml_etree.py

@@ -1594,6 +1594,83 @@ def test_recursive_repr(self):
             with self.assertRaises(RuntimeError):
                 repr(e)  # Should not crash
 
+    def test_element_get_text(self):
+        # Issue #27863
+        class X(str):
+            def __del__(self):
+                try:
+                    elem.text
+                except NameError:
+                    pass
+
+        b = ET.TreeBuilder()
+        b.start('tag', {})
+        b.data('ABCD')
+        b.data(X('EFGH'))
+        b.data('IJKL')
+        b.end('tag')
+
+        elem = b.close()
+        self.assertEqual(elem.text, 'ABCDEFGHIJKL')
+
+    def test_element_get_tail(self):
+        # Issue #27863
+        class X(str):
+            def __del__(self):
+                try:
+                    elem[0].tail
+                except NameError:
+                    pass
+
+        b = ET.TreeBuilder()
+        b.start('root', {})
+        b.start('tag', {})
+        b.end('tag')
+        b.data('ABCD')
+        b.data(X('EFGH'))
+        b.data('IJKL')
+        b.end('root')
+
+        elem = b.close()
+        self.assertEqual(elem[0].tail, 'ABCDEFGHIJKL')
+
+    def test_element_iter(self):
+        # Issue #27863
+        e = ET.Element('tag')
+        e.extend([None])  # non-Element
+
+        it = e.iter()
+        self.assertIs(next(it), e)
+        self.assertRaises((AttributeError, TypeError), list, it)
+
+    def test_subscr(self):
+        # Issue #27863
+        class X:
+            def __index__(self):
+                del e[:]
+                return 1
+
+        e = ET.Element('elem')
+        e.append(ET.Element('child'))
+        e[:X()]  # shouldn't crash
+
+        e.append(ET.Element('child'))
+        e[0:10:X()]  # shouldn't crash
+
+    def test_ass_subscr(self):
+        # Issue #27863
+        class X:
+            def __index__(self):
+                e[:] = []
+                return 1
+
+        e = ET.Element('elem')
+        for _ in range(10):
+            e.insert(0, ET.Element('child'))
+
+        e[0:10:X()] = []  # shouldn't crash
+
+
 class MutatingElementPath(str):
     def __new__(cls, elem, *args):
         self = str.__new__(cls, *args)

Misc/NEWS

@@ -42,6 +42,9 @@ Extension Modules
 Library
 -------
 
+- bpo-27863: Fixed multiple crashes in ElementTree caused by race conditions
+  and wrong types.
+
 - bpo-29942: Fix a crash in itertools.chain.from_iterable when encountering
   long runs of empty iterables.
 

Modules/_elementtree.c

@@ -164,21 +164,18 @@ deepcopy(PyObject* object, PyObject* memo)
 LOCAL(PyObject*)
 list_join(PyObject* list)
 {
-    /* join list elements (destroying the list in the process) */
-
+    /* join list elements */
     PyObject* joiner;
     PyObject* function;
     PyObject* args;
     PyObject* result;
 
     switch (PyList_GET_SIZE(list)) {
     case 0:
-        Py_DECREF(list);
         return PyString_FromString("");
     case 1:
         result = PyList_GET_ITEM(list, 0);
         Py_INCREF(result);
-        Py_DECREF(list);
         return result;
     }
 
@@ -196,9 +193,13 @@ list_join(PyObject* list)
     }
 
     args = PyTuple_New(1);
-    if (!args)
+    if (!args) {
+        Py_DECREF(function);
+        Py_DECREF(joiner);
         return NULL;
+    }
 
+    Py_INCREF(list);
     PyTuple_SET_ITEM(args, 0, list);
 
     result = PyObject_CallObject(function, args);
@@ -435,15 +436,17 @@ element_get_text(ElementObject* self)
 {
     /* return borrowed reference to text attribute */
 
-    PyObject* res = self->text;
+    PyObject *res = self->text;
 
     if (JOIN_GET(res)) {
         res = JOIN_OBJ(res);
         if (PyList_CheckExact(res)) {
-            res = list_join(res);
-            if (!res)
+            PyObject *tmp = list_join(res);
+            if (!tmp)
                 return NULL;
-            self->text = res;
+            self->text = tmp;
+            Py_DECREF(res);
+            res = tmp;
         }
     }
 
@@ -455,15 +458,17 @@ element_get_tail(ElementObject* self)
 {
     /* return borrowed reference to text attribute */
 
-    PyObject* res = self->tail;
+    PyObject *res = self->tail;
 
     if (JOIN_GET(res)) {
         res = JOIN_OBJ(res);
         if (PyList_CheckExact(res)) {
-            res = list_join(res);
-            if (!res)
+            PyObject *tmp = list_join(res);
+            if (!tmp)
                 return NULL;
-            self->tail = res;
+            self->tail = tmp;
+            Py_DECREF(res);
+            res = tmp;
         }
     }
 
@@ -1730,6 +1735,37 @@ treebuilder_dealloc(TreeBuilderObject* self)
     PyObject_Del(self);
 }
 
+/* -------------------------------------------------------------------- */
+/* helpers for handling of arbitrary element-like objects */
+
+static void
+treebuilder_set_element_text_or_tail(PyObject **data, PyObject **dest)
+{
+    PyObject *tmp = JOIN_OBJ(*dest);
+    *dest = JOIN_SET(*data, PyList_CheckExact(*data));
+    *data = NULL;
+    Py_DECREF(tmp);
+}
+
+LOCAL(void)
+treebuilder_flush_data(TreeBuilderObject* self)
+{
+    ElementObject *element = self->last;
+
+    if (self->data) {
+        if (self->this == element) {
+            treebuilder_set_element_text_or_tail(
+                &self->data,
+                &element->text);
+        }
+        else {
+            treebuilder_set_element_text_or_tail(
+                &self->data,
+                &element->tail);
+        }
+    }
+}
+
 LOCAL(int)
 treebuilder_append_event(TreeBuilderObject *self, PyObject *action,
                          PyObject *node)
@@ -1764,20 +1800,7 @@ treebuilder_handle_start(TreeBuilderObject* self, PyObject* tag,
     PyObject* node;
     PyObject* this;
 
-    if (self->data) {
-        if (self->this == self->last) {
-            Py_DECREF(JOIN_OBJ(self->last->text));
-            self->last->text = JOIN_SET(
-                self->data, PyList_CheckExact(self->data)
-                );
-        } else {
-            Py_DECREF(JOIN_OBJ(self->last->tail));
-            self->last->tail = JOIN_SET(
-                self->data, PyList_CheckExact(self->data)
-                );
-        }
-        self->data = NULL;
-    }
+    treebuilder_flush_data(self);
 
     node = element_new(tag, attrib);
     if (!node)
@@ -1867,20 +1890,7 @@ treebuilder_handle_end(TreeBuilderObject* self, PyObject* tag)
 {
     ElementObject *item;
 
-    if (self->data) {
-        if (self->this == self->last) {
-            Py_DECREF(JOIN_OBJ(self->last->text));
-            self->last->text = JOIN_SET(
-                self->data, PyList_CheckExact(self->data)
-                );
-        } else {
-            Py_DECREF(JOIN_OBJ(self->last->tail));
-            self->last->tail = JOIN_SET(
-                self->data, PyList_CheckExact(self->data)
-                );
-        }
-        self->data = NULL;
-    }
+    treebuilder_flush_data(self);
 
     if (self->index == 0) {
         PyErr_SetString(