From a4493c4524d2b38ad601150eb00b932fddbbba6c Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Thu, 31 Oct 2019 14:31:21 -0600 Subject: [PATCH 1/2] bpo-38610: Fix possible crashes in several list methods Hold strong references to list elements while calling PyObject_RichCompareBool(). --- Lib/test/test_list.py | 25 +++++++++++++++++++ .../2019-10-31-14-30-39.bpo-38610.fHdVMS.rst | 2 ++ Objects/listobject.c | 15 ++++++++--- 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-10-31-14-30-39.bpo-38610.fHdVMS.rst diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index fe4b2cd365f088..f76a259ec3253d 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -171,5 +171,30 @@ def test_preallocation(self): self.assertEqual(iter_size, sys.getsizeof(list([0] * 10))) self.assertEqual(iter_size, sys.getsizeof(list(range(10)))) + def test_count_index_remove_crashes(self): + # 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 00000000000000..0ee63bbb40dc66 --- /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 645742b801fac4..86690f764b7db4 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -2553,7 +2553,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) @@ -2580,7 +2583,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) @@ -2607,7 +2613,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) From 5090ae257f2c13b8d33d84c245d04fc031cca5f7 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Sun, 29 Dec 2019 23:12:57 -0700 Subject: [PATCH 2/2] Add the issue number to the test comment. --- Lib/test/test_list.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index f76a259ec3253d..b10a833033f159 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -172,8 +172,9 @@ def test_preallocation(self): self.assertEqual(iter_size, sys.getsizeof(list(range(10)))) def test_count_index_remove_crashes(self): - # The count(), index(), and remove() methods were not holding strong - # references to list elements while calling PyObject_RichCompareBool(). + # 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()