https://github.com/python/cpython/commit/c5b99f5c2c5347d66b9da362773969c531fb6c85 commit: c5b99f5c2c5347d66b9da362773969c531fb6c85 branch: main author: Bénédikt Tran <[email protected]> committer: kumaraditya303 <[email protected]> date: 2024-10-25T23:45:09+05:30 summary:
gh-125969: fix OOB in `future_schedule_callbacks` due to an evil `call_soon` (#125970) Co-authored-by: Andrew Svetlov <[email protected]> files: A Misc/NEWS.d/next/Library/2024-10-25-11-13-24.gh-issue-125969.YvbrTr.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 b3517407e5394f..4e3efa7ad6abc0 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -947,6 +947,39 @@ def __eq__(self, other): fut.remove_done_callback(evil()) + def test_evil_call_soon_list_mutation(self): + called_on_fut_callback0 = False + + pad = lambda: ... + + def evil_call_soon(*args, **kwargs): + nonlocal called_on_fut_callback0 + if called_on_fut_callback0: + # Called when handling fut->fut_callbacks[0] + # and mutates the length fut->fut_callbacks. + fut.remove_done_callback(int) + fut.remove_done_callback(pad) + else: + called_on_fut_callback0 = True + + fake_event_loop = lambda: ... + 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() + self.assertIs(fut.get_loop(), fake_event_loop) + + fut.add_done_callback(str) # sets fut->fut_callback0 + fut.add_done_callback(int) # sets fut->fut_callbacks[0] + fut.add_done_callback(pad) # sets fut->fut_callbacks[1] + fut.add_done_callback(pad) # sets fut->fut_callbacks[2] + fut.set_result("boom") + + # When there are no more callbacks, the Python implementation + # returns an empty list but the C implementation returns None. + self.assertIn(fut._callbacks, (None, [])) + @unittest.skipUnless(hasattr(futures, '_CFuture'), 'requires the C _asyncio module') diff --git a/Misc/NEWS.d/next/Library/2024-10-25-11-13-24.gh-issue-125969.YvbrTr.rst b/Misc/NEWS.d/next/Library/2024-10-25-11-13-24.gh-issue-125969.YvbrTr.rst new file mode 100644 index 00000000000000..dc99adff7416c5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-10-25-11-13-24.gh-issue-125969.YvbrTr.rst @@ -0,0 +1,2 @@ +Fix an out-of-bounds crash when an evil :meth:`asyncio.loop.call_soon` +mutates the length of the internal callbacks list. Patch by Bénédikt Tran. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index a7385b4c541df9..91ba5cea880a06 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -406,9 +406,6 @@ future_ensure_alive(FutureObj *fut) static int future_schedule_callbacks(asyncio_state *state, FutureObj *fut) { - Py_ssize_t len; - Py_ssize_t i; - if (fut->fut_callback0 != NULL) { /* There's a 1st callback */ @@ -434,27 +431,25 @@ future_schedule_callbacks(asyncio_state *state, FutureObj *fut) return 0; } - len = PyList_GET_SIZE(fut->fut_callbacks); - if (len == 0) { - /* The list of callbacks was empty; clear it and return. */ - Py_CLEAR(fut->fut_callbacks); - return 0; - } - - for (i = 0; i < len; i++) { - PyObject *cb_tup = PyList_GET_ITEM(fut->fut_callbacks, i); + // Beware: An evil call_soon could change fut->fut_callbacks. + // The idea is to transfer the ownership of the callbacks list + // so that external code is not able to mutate the list during + // the iteration. + PyObject *callbacks = fut->fut_callbacks; + fut->fut_callbacks = NULL; + Py_ssize_t n = PyList_GET_SIZE(callbacks); + for (Py_ssize_t i = 0; i < n; i++) { + assert(PyList_GET_SIZE(callbacks) == n); + PyObject *cb_tup = PyList_GET_ITEM(callbacks, i); PyObject *cb = PyTuple_GET_ITEM(cb_tup, 0); PyObject *ctx = PyTuple_GET_ITEM(cb_tup, 1); if (call_soon(state, fut->fut_loop, cb, (PyObject *)fut, ctx)) { - /* If an error occurs in pure-Python implementation, - all callbacks are cleared. */ - Py_CLEAR(fut->fut_callbacks); + Py_DECREF(callbacks); return -1; } } - - Py_CLEAR(fut->fut_callbacks); + Py_DECREF(callbacks); return 0; } _______________________________________________ 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]
