https://github.com/python/cpython/commit/f819d4301d7c75f02be1187fda017f0e7b608816
commit: f819d4301d7c75f02be1187fda017f0e7b608816
branch: main
author: Bénédikt Tran <[email protected]>
committer: kumaraditya303 <[email protected]>
date: 2024-10-27T20:34:43+05:30
summary:
gh-125984: fix use-after-free on `fut->fut_{callback,context}0` due to an evil
`loop.__getattribute__` (#126003)
files:
A Misc/NEWS.d/next/Library/2024-10-26-12-50-48.gh-issue-125984.d4vp5_.rst
M Lib/test/test_asyncio/test_futures.py
M Modules/_asynciomodule.c
diff --git a/Lib/test/test_asyncio/test_futures.py
b/Lib/test/test_asyncio/test_futures.py
index 4e3efa7ad6abc0..bac76b074981df 100644
--- a/Lib/test/test_asyncio/test_futures.py
+++ b/Lib/test/test_asyncio/test_futures.py
@@ -32,6 +32,25 @@ def last_cb():
pass
+class ReachableCode(Exception):
+ """Exception to raise to indicate that some code was reached.
+
+ Use this exception if using mocks is not a good alternative.
+ """
+
+
+class SimpleEvilEventLoop(asyncio.base_events.BaseEventLoop):
+ """Base class for UAF and other evil stuff requiring an evil event loop."""
+
+ def get_debug(self): # to suppress tracebacks
+ return False
+
+ def __del__(self):
+ # Automatically close the evil event loop to avoid warnings.
+ if not self.is_closed() and not self.is_running():
+ self.close()
+
+
class DuckFuture:
# Class that does not inherit from Future but aims to be duck-type
# compatible with it.
@@ -948,6 +967,7 @@ def __eq__(self, other):
fut.remove_done_callback(evil())
def test_evil_call_soon_list_mutation(self):
+ # see: https://github.com/python/cpython/issues/125969
called_on_fut_callback0 = False
pad = lambda: ...
@@ -962,9 +982,8 @@ def evil_call_soon(*args, **kwargs):
else:
called_on_fut_callback0 = True
- fake_event_loop = lambda: ...
+ fake_event_loop = SimpleEvilEventLoop()
fake_event_loop.call_soon = evil_call_soon
- fake_event_loop.get_debug = lambda: False # suppress traceback
with mock.patch.object(self, 'loop', fake_event_loop):
fut = self._new_future()
@@ -980,6 +999,56 @@ 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__getattribute__(self):
+ # see: https://github.com/python/cpython/issues/125984
+
+ class EvilEventLoop(SimpleEvilEventLoop):
+ def call_soon(self, *args, **kwargs):
+ super().call_soon(*args, **kwargs)
+ raise ReachableCode
+
+ def __getattribute__(self, name):
+ nonlocal fut_callback_0
+ if name == 'call_soon':
+ fut.remove_done_callback(fut_callback_0)
+ del fut_callback_0
+ return object.__getattribute__(self, name)
+
+ evil_loop = EvilEventLoop()
+ with mock.patch.object(self, 'loop', evil_loop):
+ fut = self._new_future()
+ self.assertIs(fut.get_loop(), evil_loop)
+
+ fut_callback_0 = lambda: ...
+ fut.add_done_callback(fut_callback_0)
+ self.assertRaises(ReachableCode, fut.set_result, "boom")
+
+ def test_use_after_free_on_fut_context_0_with_evil__getattribute__(self):
+ # see: https://github.com/python/cpython/issues/125984
+
+ class EvilEventLoop(SimpleEvilEventLoop):
+ def call_soon(self, *args, **kwargs):
+ super().call_soon(*args, **kwargs)
+ raise ReachableCode
+
+ def __getattribute__(self, name):
+ if name == 'call_soon':
+ # resets the future's event loop
+ fut.__init__(loop=SimpleEvilEventLoop())
+ return object.__getattribute__(self, name)
+
+ evil_loop = EvilEventLoop()
+ with mock.patch.object(self, 'loop', evil_loop):
+ fut = self._new_future()
+ self.assertIs(fut.get_loop(), evil_loop)
+
+ fut_callback_0 = mock.Mock()
+ fut_context_0 = mock.Mock()
+ fut.add_done_callback(fut_callback_0, context=fut_context_0)
+ del fut_context_0
+ del fut_callback_0
+ self.assertRaises(ReachableCode, fut.set_result, "boom")
+
@unittest.skipUnless(hasattr(futures, '_CFuture'),
'requires the C _asyncio module')
diff --git
a/Misc/NEWS.d/next/Library/2024-10-26-12-50-48.gh-issue-125984.d4vp5_.rst
b/Misc/NEWS.d/next/Library/2024-10-26-12-50-48.gh-issue-125984.d4vp5_.rst
new file mode 100644
index 00000000000000..7a1d7b53b11301
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-10-26-12-50-48.gh-issue-125984.d4vp5_.rst
@@ -0,0 +1,3 @@
+Fix use-after-free crashes on :class:`asyncio.Future` objects for which the
+underlying event loop implements an evil :meth:`~object.__getattribute__`.
+Reported by Nico-Posada. Patch by Bénédikt Tran.
diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c
index 91ba5cea880a06..a4dab29ee9428d 100644
--- a/Modules/_asynciomodule.c
+++ b/Modules/_asynciomodule.c
@@ -409,12 +409,19 @@ future_schedule_callbacks(asyncio_state *state, FutureObj
*fut)
if (fut->fut_callback0 != NULL) {
/* There's a 1st callback */
- int ret = call_soon(state,
- fut->fut_loop, fut->fut_callback0,
- (PyObject *)fut, fut->fut_context0);
-
- Py_CLEAR(fut->fut_callback0);
- Py_CLEAR(fut->fut_context0);
+ // Beware: An evil call_soon could alter fut_callback0 or fut_context0.
+ // Since we are anyway clearing them after the call, whether call_soon
+ // succeeds or not, the idea is to transfer ownership so that external
+ // code is not able to alter them during the call.
+ PyObject *fut_callback0 = fut->fut_callback0;
+ fut->fut_callback0 = NULL;
+ PyObject *fut_context0 = fut->fut_context0;
+ fut->fut_context0 = NULL;
+
+ int ret = call_soon(state, fut->fut_loop, fut_callback0,
+ (PyObject *)fut, fut_context0);
+ Py_CLEAR(fut_callback0);
+ Py_CLEAR(fut_context0);
if (ret) {
/* If an error occurs in pure-Python implementation,
all callbacks are cleared. */
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
