[Python-checkins] [3.7] bpo-38610: Fix possible crashes in several list methods (GH-17022) (GH-17759)

[Pablo Galindo]

https://github.com/python/cpython/commit/296d45ec10fb55532bc3fac2311a3f91299ecf59
commit: 296d45ec10fb55532bc3fac2311a3f91299ecf59
branch: 3.7
author: Pablo Galindo <Pablogsal at gmail.com>
committer: GitHub <noreply at github.com>
date: 2019-12-30T19:58:31Z
summary:

[3.7] bpo-38610: Fix possible crashes in several list methods (GH-17022) (GH-17759)

Hold strong references to list elements while calling PyObject_RichCompareBool()..
(cherry picked from commit d9e561d23d994e3ed15f4fcbd7ee5c8fe50f190b)

Co-authored-by: Zackery Spytz <zspytz at gmail.com>

Co-authored-by: Zackery Spytz <zspytz at gmail.com>

files:
A Misc/NEWS.d/next/Core and Builtins/2019-10-31-14-30-39.bpo-38610.fHdVMS.rst
M Lib/test/test_list.py
M Objects/listobject.c

diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py
index 5078d4bc1ced5..ece4598e4eaf2 100644
--- a/Lib/test/test_list.py
+++ b/Lib/test/test_list.py
@@ -162,5 +162,31 @@ class L(list): pass
         with self.assertRaises(TypeError):
             (3,) + L([1,2])
 
+    def test_count_index_remove_crashes(self):
+        # bpo-38610: The count(), index(), and remove() methods were not
+        # holding strong references to list elements while calling
+        # PyObject_RichCompareBool().
+        class X:
+            def __eq__(self, other):
+                lst.clear()
+                return NotImplemented
+
+        lst = [X()]
+        with self.assertRaises(ValueError):
+            lst.index(lst)
+
+        class L(list):
+            def __eq__(self, other):
+                str(other)
+                return NotImplemented
+
+        lst = L([X()])
+        lst.count(lst)
+
+        lst = L([X()])
+        with self.assertRaises(ValueError):
+            lst.remove(lst)
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-10-31-14-30-39.bpo-38610.fHdVMS.rst b/Misc/NEWS.d/next/Core and Builtins/2019-10-31-14-30-39.bpo-38610.fHdVMS.rst
new file mode 100644
index 0000000000000..0ee63bbb40dc6
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-10-31-14-30-39.bpo-38610.fHdVMS.rst	
@@ -0,0 +1,2 @@
+Fix possible crashes in several list methods by holding strong references to
+list elements when calling :c:func:`PyObject_RichCompareBool`.
diff --git a/Objects/listobject.c b/Objects/listobject.c
index c5e7553efcf90..724f25677a16e 100644
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -2506,7 +2506,10 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start,
             stop = 0;
     }
     for (i = start; i < stop && i < Py_SIZE(self); i++) {
-        int cmp = PyObject_RichCompareBool(self->ob_item[i], value, Py_EQ);
+        PyObject *obj = self->ob_item[i];
+        Py_INCREF(obj);
+        int cmp = PyObject_RichCompareBool(obj, value, Py_EQ);
+        Py_DECREF(obj);
         if (cmp > 0)
             return PyLong_FromSsize_t(i);
         else if (cmp < 0)
@@ -2533,7 +2536,10 @@ list_count(PyListObject *self, PyObject *value)
     Py_ssize_t i;
 
     for (i = 0; i < Py_SIZE(self); i++) {
-        int cmp = PyObject_RichCompareBool(self->ob_item[i], value, Py_EQ);
+        PyObject *obj = self->ob_item[i];
+        Py_INCREF(obj);
+        int cmp = PyObject_RichCompareBool(obj, value, Py_EQ);
+        Py_DECREF(obj);
         if (cmp > 0)
             count++;
         else if (cmp < 0)
@@ -2560,7 +2566,10 @@ list_remove(PyListObject *self, PyObject *value)
     Py_ssize_t i;
 
     for (i = 0; i < Py_SIZE(self); i++) {
-        int cmp = PyObject_RichCompareBool(self->ob_item[i], value, Py_EQ);
+        PyObject *obj = self->ob_item[i];
+        Py_INCREF(obj);
+        int cmp = PyObject_RichCompareBool(obj, value, Py_EQ);
+        Py_DECREF(obj);
         if (cmp > 0) {
             if (list_ass_slice(self, i, i+1,
                                (PyObject *)NULL) == 0)