From 7f609280d6cdba1841cf4ebe7188dcff8de48f79 Mon Sep 17 00:00:00 2001 From: Frederick Price Date: Mon, 1 Apr 2024 21:50:37 -0400 Subject: [PATCH 1/2] CVE-2022-48560 Cherry-pick c563f409ea30bcb0623d785428c9257917371b76 Cherry-pick c563f409ea30bcb0623d785428c9257917371b76 Fix up conflicts --- Lib/test/test_heapq.py | 31 +++++++++++++++++ .../2020-01-22-15-53-37.bpo-39421.O3nG7u.rst | 2 ++ Modules/_heapqmodule.c | 33 +++++++++++++++---- 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-01-22-15-53-37.bpo-39421.O3nG7u.rst diff --git a/Lib/test/test_heapq.py b/Lib/test/test_heapq.py index c4de593bb820a8..f089a7c20e8d51 100644 --- a/Lib/test/test_heapq.py +++ b/Lib/test/test_heapq.py @@ -396,6 +396,37 @@ def test_heappop_mutating_heap(self): with self.assertRaises((IndexError, RuntimeError)): self.module.heappop(heap) + def test_comparison_operator_modifiying_heap(self): + # See bpo-39421: Strong references need to be taken + # when comparing objects as they can alter the heap + class EvilClass(int): + def __lt__(self, o): + heap.clear() + return NotImplemented + + heap = [] + self.module.heappush(heap, EvilClass(0)) + self.assertRaises(IndexError, self.module.heappushpop, heap, 1) + + def test_comparison_operator_modifiying_heap_two_heaps(self): + + class h(int): + def __lt__(self, o): + list2.clear() + return NotImplemented + + class g(int): + def __lt__(self, o): + list1.clear() + return NotImplemented + + list1, list2 = [], [] + + self.module.heappush(list1, h(0)) + self.module.heappush(list2, g(0)) + + self.assertRaises((IndexError, RuntimeError), self.module.heappush, list1, g(1)) + self.assertRaises((IndexError, RuntimeError), self.module.heappush, list2, h(1)) class TestErrorHandlingPython(TestErrorHandling): module = py_heapq diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-01-22-15-53-37.bpo-39421.O3nG7u.rst b/Misc/NEWS.d/next/Core and Builtins/2020-01-22-15-53-37.bpo-39421.O3nG7u.rst new file mode 100644 index 00000000000000..bae008150ee127 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-01-22-15-53-37.bpo-39421.O3nG7u.rst @@ -0,0 +1,2 @@ +Fix possible crashes when operating with the functions in the :mod:`heapq` +module and custom comparison operators. diff --git a/Modules/_heapqmodule.c b/Modules/_heapqmodule.c index 5b0ef691545ba5..9e2c5c784cb30d 100644 --- a/Modules/_heapqmodule.c +++ b/Modules/_heapqmodule.c @@ -52,7 +52,11 @@ _siftdown(PyListObject *heap, Py_ssize_t startpos, Py_ssize_t pos) while (pos > startpos) { parentpos = (pos - 1) >> 1; parent = PyList_GET_ITEM(heap, parentpos); + Py_INCREF(newitem); + Py_INCREF(parent); cmp = cmp_lt(newitem, parent); + Py_DECREF(parent); + Py_DECREF(newitem); if (cmp == -1) return -1; if (size != PyList_GET_SIZE(heap)) { @@ -93,9 +97,13 @@ _siftup(PyListObject *heap, Py_ssize_t pos) childpos = 2*pos + 1; /* leftmost child position */ rightpos = childpos + 1; if (rightpos < endpos) { - cmp = cmp_lt( - PyList_GET_ITEM(heap, childpos), - PyList_GET_ITEM(heap, rightpos)); + PyObject* a = PyList_GET_ITEM(heap, childpos); + PyObject* b = PyList_GET_ITEM(heap, rightpos); + Py_INCREF(a); + Py_INCREF(b); + cmp = cmp_lt(a,b); + Py_DECREF(a); + Py_DECREF(b); if (cmp == -1) return -1; if (cmp == 0) @@ -236,7 +244,10 @@ heappushpop(PyObject *self, PyObject *args) return item; } - cmp = cmp_lt(PyList_GET_ITEM(heap, 0), item); + PyObject* top = PyList_GET_ITEM(heap, 0); + Py_INCREF(top); + cmp = cmp_lt(top, item); + Py_DECREF(top); if (cmp == -1) return NULL; if (cmp == 0) { @@ -395,7 +406,11 @@ _siftdownmax(PyListObject *heap, Py_ssize_t startpos, Py_ssize_t pos) while (pos > startpos){ parentpos = (pos - 1) >> 1; parent = PyList_GET_ITEM(heap, parentpos); + Py_INCREF(parent); + Py_INCREF(newitem); cmp = cmp_lt(parent, newitem); + Py_DECREF(parent); + Py_DECREF(newitem); if (cmp == -1) { Py_DECREF(newitem); return -1; @@ -436,9 +451,13 @@ _siftupmax(PyListObject *heap, Py_ssize_t pos) childpos = 2*pos + 1; /* leftmost child position */ rightpos = childpos + 1; if (rightpos < endpos) { - cmp = cmp_lt( - PyList_GET_ITEM(heap, rightpos), - PyList_GET_ITEM(heap, childpos)); + PyObject* a = PyList_GET_ITEM(heap, rightpos); + PyObject* b = PyList_GET_ITEM(heap, childpos); + Py_INCREF(a); + Py_INCREF(b); + cmp = cmp_lt(a, b); + Py_DECREF(a); + Py_DECREF(b); if (cmp == -1) { Py_DECREF(newitem); return -1; From 7b7297899dc967815663bf6169d08592d7f92381 Mon Sep 17 00:00:00 2001 From: Frederick Price Date: Tue, 2 Apr 2024 11:24:31 -0400 Subject: [PATCH 2/2] CVE-2022-48560 Change code to be compatible with Python2 --- Lib/test/test_heapq.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_heapq.py b/Lib/test/test_heapq.py index f089a7c20e8d51..0f2990971184a2 100644 --- a/Lib/test/test_heapq.py +++ b/Lib/test/test_heapq.py @@ -401,7 +401,8 @@ def test_comparison_operator_modifiying_heap(self): # when comparing objects as they can alter the heap class EvilClass(int): def __lt__(self, o): - heap.clear() + # heap.clear() + del heap[:] return NotImplemented heap = [] @@ -412,12 +413,14 @@ def test_comparison_operator_modifiying_heap_two_heaps(self): class h(int): def __lt__(self, o): - list2.clear() + # list2.clear() + del list2[:] return NotImplemented class g(int): def __lt__(self, o): - list1.clear() + # list1.clear() + del list1[:] return NotImplemented list1, list2 = [], []