From 256bc2b9a1d5c01af8186e6883ae475eb28bc4b0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 10 Jun 2024 10:43:17 +0300 Subject: [PATCH 1/3] gh-120298: Fix use-after-free in `list_richcompare_impl` --- Lib/test/test_bisect.py | 10 ++++++++++ Lib/test/test_deque.py | 12 ++++++++++++ .../2024-06-10-10-42-48.gh-issue-120298.napREA.rst | 2 ++ Objects/listobject.c | 9 ++++++++- 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-06-10-10-42-48.gh-issue-120298.napREA.rst diff --git a/Lib/test/test_bisect.py b/Lib/test/test_bisect.py index 97204d4cad3871..363fadbc59734f 100644 --- a/Lib/test/test_bisect.py +++ b/Lib/test/test_bisect.py @@ -291,6 +291,16 @@ def __gt__(self, other): self.assertEqual(i1, 40) self.assertEqual(i2, 41) + def test_use_after_free_gh120298(self): + class evil(object): + def __lt__(self, other): + other.clear() + return NotImplemented + + a = [[evil()]] + with self.assertRaises(TypeError): + self.module.insort_left(a, a) + class TestBisectPython(TestBisect, unittest.TestCase): module = py_bisect diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py index 4679f297fd7f4a..21cf2946bd2df7 100644 --- a/Lib/test/test_deque.py +++ b/Lib/test/test_deque.py @@ -782,6 +782,18 @@ def test_runtime_error_on_empty_deque(self): d.append(10) self.assertRaises(RuntimeError, next, it) + def test_use_after_free_gh120298(self): + class evil(object): + def __lt__(self, other): + other.pop() + return NotImplemented + + a = [[[evil()]]] + b = deque(a[0]) + c = deque(a) + with self.assertRaises(TypeError): + b < c + class Deque(deque): pass diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-10-10-42-48.gh-issue-120298.napREA.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-10-10-42-48.gh-issue-120298.napREA.rst new file mode 100644 index 00000000000000..531d39517ac423 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-10-10-42-48.gh-issue-120298.napREA.rst @@ -0,0 +1,2 @@ +Fix use-after free in ``list_richcompare_impl`` which can be invoked via +some specificly tailored evil input. diff --git a/Objects/listobject.c b/Objects/listobject.c index d09bb6391034d1..6829d5d28656cf 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3382,7 +3382,14 @@ list_richcompare_impl(PyObject *v, PyObject *w, int op) } /* Compare the final item again using the proper operator */ - return PyObject_RichCompare(vl->ob_item[i], wl->ob_item[i], op); + PyObject *vitem = vl->ob_item[i]; + PyObject *witem = wl->ob_item[i]; + Py_INCREF(vitem); + Py_INCREF(witem); + PyObject *result = PyObject_RichCompare(vl->ob_item[i], wl->ob_item[i], op); + Py_DECREF(vitem); + Py_DECREF(witem); + return result; } static PyObject * From 28abb103bdf16506d8eb65603b01ebf3a54009cd Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 10 Jun 2024 16:42:59 +0300 Subject: [PATCH 2/3] Address review --- Lib/test/test_bisect.py | 10 ---------- Lib/test/test_deque.py | 12 ------------ Lib/test/test_list.py | 11 +++++++++++ 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_bisect.py b/Lib/test/test_bisect.py index 363fadbc59734f..97204d4cad3871 100644 --- a/Lib/test/test_bisect.py +++ b/Lib/test/test_bisect.py @@ -291,16 +291,6 @@ def __gt__(self, other): self.assertEqual(i1, 40) self.assertEqual(i2, 41) - def test_use_after_free_gh120298(self): - class evil(object): - def __lt__(self, other): - other.clear() - return NotImplemented - - a = [[evil()]] - with self.assertRaises(TypeError): - self.module.insort_left(a, a) - class TestBisectPython(TestBisect, unittest.TestCase): module = py_bisect diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py index 21cf2946bd2df7..4679f297fd7f4a 100644 --- a/Lib/test/test_deque.py +++ b/Lib/test/test_deque.py @@ -782,18 +782,6 @@ def test_runtime_error_on_empty_deque(self): d.append(10) self.assertRaises(RuntimeError, next, it) - def test_use_after_free_gh120298(self): - class evil(object): - def __lt__(self, other): - other.pop() - return NotImplemented - - a = [[[evil()]]] - b = deque(a[0]) - c = deque(a) - with self.assertRaises(TypeError): - b < c - class Deque(deque): pass diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 0601b33e79ebb6..6d6238d29d783e 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -234,6 +234,17 @@ def __eq__(self, other): list4 = [1] self.assertFalse(list3 == list4) + def test_lt_operator_modifying_operand(self): + # See gh-120298 + class evil(object): + def __lt__(self, other): + other.clear() + return NotImplemented + + a = [[evil()]] + with self.assertRaises(TypeError): + a[0] < a + @cpython_only def test_preallocation(self): iterable = [0] * 10 From 6c56bc01f5c67a8a7456f1b09129963f04f0c71a Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Mon, 10 Jun 2024 18:39:12 +0300 Subject: [PATCH 3/3] Update Lib/test/test_list.py Co-authored-by: Serhiy Storchaka --- Lib/test/test_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 6d6238d29d783e..d21429fae09b37 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -236,7 +236,7 @@ def __eq__(self, other): def test_lt_operator_modifying_operand(self): # See gh-120298 - class evil(object): + class evil: def __lt__(self, other): other.clear() return NotImplemented