https://github.com/python/cpython/commit/54bbb5e3363ef3634f1fd7521ba3f3c42c8... commit: 54bbb5e3363ef3634f1fd7521ba3f3c42c81a331 branch: 3.10 author: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> committer: miss-islington <31488909+miss-islington@users.noreply.github.com> date: 2022-09-30T13:28:27-07:00 summary: GH-97592: Fix crash in C remove_done_callback due to evil code (GH-97660) Evil code could cause fut_callbacks to be cleared when PyObject_RichCompareBool is called. (cherry picked from commit 63780f4599acc2c5ee8af5f37ab76c162ad21065) Co-authored-by: Guido van Rossum <guido@python.org> files: A Misc/NEWS.d/next/Library/2022-09-29-23-22-24.gh-issue-97592.tpJg_J.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 838147b1e65f..012674260370 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -827,6 +827,21 @@ def __eq__(self, other): fut.remove_done_callback(evil()) + def test_remove_done_callbacks_list_clear(self): + # see https://github.com/python/cpython/issues/97592 for details + + fut = self._new_future() + fut.add_done_callback(str) + + for _ in range(63): + fut.add_done_callback(id) + + class evil: + def __eq__(self, other): + fut.remove_done_callback(other) + + fut.remove_done_callback(evil()) + def test_schedule_callbacks_list_mutation_1(self): # see http://bugs.python.org/issue28963 for details diff --git a/Misc/NEWS.d/next/Library/2022-09-29-23-22-24.gh-issue-97592.tpJg_J.rst b/Misc/NEWS.d/next/Library/2022-09-29-23-22-24.gh-issue-97592.tpJg_J.rst new file mode 100644 index 000000000000..aa245cf94400 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-09-29-23-22-24.gh-issue-97592.tpJg_J.rst @@ -0,0 +1 @@ +Avoid a crash in the C version of :meth:`asyncio.Future.remove_done_callback` when an evil argument is passed. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 1f6de2177a37..c627382a53f0 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1042,7 +1042,11 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) return NULL; } - for (i = 0; i < PyList_GET_SIZE(self->fut_callbacks); i++) { + // Beware: PyObject_RichCompareBool below may change fut_callbacks. + // See GH-97592. + for (i = 0; + self->fut_callbacks != NULL && i < PyList_GET_SIZE(self->fut_callbacks); + i++) { int ret; PyObject *item = PyList_GET_ITEM(self->fut_callbacks, i); Py_INCREF(item); @@ -1061,7 +1065,8 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) } } - if (j == 0) { + // Note: fut_callbacks may have been cleared. + if (j == 0 || self->fut_callbacks == NULL) { Py_CLEAR(self->fut_callbacks); Py_DECREF(newlist); return PyLong_FromSsize_t(len + cleared_callback0);