From 208865b5c64fed389db45e85c6618cf05f0f6aa7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?=
 <10796600+picnixz@users.noreply.github.com>
Date: Sun, 27 Oct 2024 18:10:10 +0100
Subject: [PATCH] gh-125966: fix use-after-free on `fut->fut_callback0` due to
 an evil callback's `__eq__` in asyncio (GH-125967) (cherry picked from commit
 ed5059eeb1aa50b481957307db5a34b937497382)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
---
 Lib/test/test_asyncio/test_futures.py          | 18 ++++++++++++++++++
 ...4-10-25-10-53-56.gh-issue-125966.eOCYU_.rst |  2 ++
 Modules/_asynciomodule.c                       |  7 ++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 Misc/NEWS.d/next/Library/2024-10-25-10-53-56.gh-issue-125966.eOCYU_.rst

diff --git a/Lib/test/test_asyncio/test_futures.py b/Lib/test/test_asyncio/test_futures.py
index 63804256ec7726..8be6dcac548ec0 100644
--- a/Lib/test/test_asyncio/test_futures.py
+++ b/Lib/test/test_asyncio/test_futures.py
@@ -988,6 +988,24 @@ def evil_call_soon(*args, **kwargs):
             # returns an empty list but the C implementation returns None.
             self.assertIn(fut._callbacks, (None, []))
 
+    def test_use_after_free_on_fut_callback_0_with_evil__eq__(self):
+        # Special thanks to Nico-Posada for the original PoC.
+        # See https://github.com/python/cpython/issues/125966.
+
+        fut = self._new_future()
+
+        class cb_pad:
+            def __eq__(self, other):
+                return True
+
+        class evil(cb_pad):
+            def __eq__(self, other):
+                fut.remove_done_callback(None)
+                return NotImplemented
+
+        fut.add_done_callback(cb_pad())
+        fut.remove_done_callback(evil())
+
     def test_use_after_free_on_fut_callback_0_with_evil__getattribute__(self):
         # see: https://github.com/python/cpython/issues/125984
 
diff --git a/Misc/NEWS.d/next/Library/2024-10-25-10-53-56.gh-issue-125966.eOCYU_.rst b/Misc/NEWS.d/next/Library/2024-10-25-10-53-56.gh-issue-125966.eOCYU_.rst
new file mode 100644
index 00000000000000..9fe8795de18003
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-10-25-10-53-56.gh-issue-125966.eOCYU_.rst
@@ -0,0 +1,2 @@
+Fix a use-after-free crash in :meth:`asyncio.Future.remove_done_callback`.
+Patch by Bénédikt Tran.
diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c
index 8787fe4ac1cc51..9016c077e0d657 100644
--- a/Modules/_asynciomodule.c
+++ b/Modules/_asynciomodule.c
@@ -1044,7 +1044,12 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
     ENSURE_FUTURE_ALIVE(state, self)
 
     if (self->fut_callback0 != NULL) {
-        int cmp = PyObject_RichCompareBool(self->fut_callback0, fn, Py_EQ);
+        // Beware: An evil PyObject_RichCompareBool could free fut_callback0
+        // before a recursive call is made with that same arg. For details, see
+        // https://github.com/python/cpython/pull/125967#discussion_r1816593340.
+        PyObject *fut_callback0 = Py_NewRef(self->fut_callback0);
+        int cmp = PyObject_RichCompareBool(fut_callback0, fn, Py_EQ);
+        Py_DECREF(fut_callback0);
         if (cmp == -1) {
             return NULL;
         }