From ed635070cc2e144e0b0dbe74c527ffb39bbf84c6 Mon Sep 17 00:00:00 2001 From: Nico-Posada Date: Fri, 1 Nov 2024 14:43:13 -0400 Subject: [PATCH 1/3] fix uaf and add blurb --- ...-11-01-14-31-41.gh-issue-126138.yTniOG.rst | 3 +++ Modules/_asynciomodule.c | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst diff --git a/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst b/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst new file mode 100644 index 00000000000000..41f3134938ca31 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst @@ -0,0 +1,3 @@ +Fix a use-after-free bug in :class:`asyncio.Task` caused by the coroutine +yielding an evil object that could prematurely free objects when the task +would try to call back to python functions. Patch by Nico Posada. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index d4135f04e56575..5e09fcb8110c95 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -2120,7 +2120,7 @@ _asyncio_Task___init___impl(TaskObj *self, PyObject *coro, PyObject *loop, return -1; } } else { - self->task_context = Py_NewRef(context); + Py_XSETREF(self->task_context, Py_NewRef(context)); } Py_CLEAR(self->task_fut_waiter); @@ -2963,8 +2963,15 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu if (task->task_must_cancel) { PyObject *r; int is_true; + + // Beware: task->task_cancel_msg may be mutated and deleted + // prematurely in an evil `__getattribute__` function before being sent to `cancel` + // See https://github.com/python/cpython/issues/126138 + PyObject* task_cancel_msg = Py_NewRef(task->task_cancel_msg); r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), - task->task_cancel_msg); + task_cancel_msg); + Py_DECREF(task_cancel_msg); + if (r == NULL) { return NULL; } @@ -3056,8 +3063,15 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu if (task->task_must_cancel) { PyObject *r; int is_true; + + // Beware: task->task_cancel_msg may be mutated and deleted + // prematurely in an evil `__getattribute__` function before being sent to `cancel` + // See https://github.com/python/cpython/issues/126138 + PyObject* task_cancel_msg = Py_NewRef(task->task_cancel_msg); r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), - task->task_cancel_msg); + task_cancel_msg); + Py_DECREF(task_cancel_msg); + if (r == NULL) { return NULL; } From eddc04465a3e0c3827244858ef3a9d5d7074f3bf Mon Sep 17 00:00:00 2001 From: Nico-Posada Date: Fri, 1 Nov 2024 15:43:49 -0400 Subject: [PATCH 2/3] Address picnixz's changes --- ...-11-01-14-31-41.gh-issue-126138.yTniOG.rst | 6 ++--- Modules/_asynciomodule.c | 22 ++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst b/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst index 41f3134938ca31..459eebc82bd42a 100644 --- a/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst +++ b/Misc/NEWS.d/next/Library/2024-11-01-14-31-41.gh-issue-126138.yTniOG.rst @@ -1,3 +1,3 @@ -Fix a use-after-free bug in :class:`asyncio.Task` caused by the coroutine -yielding an evil object that could prematurely free objects when the task -would try to call back to python functions. Patch by Nico Posada. +Fix a use-after-free crash on :class:`asyncio.Task` objects +whose underlying coroutine yields an object that implements +an evil :meth:`~object.__getattribute__`. Patch by Nico Posada. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index c8f45bef346b10..ad21c82b7d5d89 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -2968,12 +2968,13 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu PyObject *r; int is_true; - // Beware: task->task_cancel_msg may be mutated and deleted - // prematurely in an evil `__getattribute__` function before being sent to `cancel` + // Beware: An evil `__getattribute__` could + // prematurely delete task->task_cancel_msg before the + // task is cancelled, thereby causing a UAF crash. + // // See https://github.com/python/cpython/issues/126138 - PyObject* task_cancel_msg = Py_NewRef(task->task_cancel_msg); - r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), - task_cancel_msg); + PyObject *task_cancel_msg = Py_NewRef(task->task_cancel_msg); + r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), task_cancel_msg); Py_DECREF(task_cancel_msg); if (r == NULL) { @@ -3068,12 +3069,13 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu PyObject *r; int is_true; - // Beware: task->task_cancel_msg may be mutated and deleted - // prematurely in an evil `__getattribute__` function before being sent to `cancel` + // Beware: An evil `__getattribute__` could + // prematurely delete task->task_cancel_msg before the + // task is cancelled, thereby causing a UAF crash. + // // See https://github.com/python/cpython/issues/126138 - PyObject* task_cancel_msg = Py_NewRef(task->task_cancel_msg); - r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), - task_cancel_msg); + PyObject *task_cancel_msg = Py_NewRef(task->task_cancel_msg); + r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), task_cancel_msg); Py_DECREF(task_cancel_msg); if (r == NULL) { From b4f41874efba4cde011e10f216384ddfe5f724fb Mon Sep 17 00:00:00 2001 From: Nico-Posada Date: Fri, 1 Nov 2024 15:53:21 -0400 Subject: [PATCH 3/3] Update to keep line length under 80 characters --- Modules/_asynciomodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index ad21c82b7d5d89..214c18e966c4c1 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -2974,7 +2974,8 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu // // See https://github.com/python/cpython/issues/126138 PyObject *task_cancel_msg = Py_NewRef(task->task_cancel_msg); - r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), task_cancel_msg); + r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), + task_cancel_msg); Py_DECREF(task_cancel_msg); if (r == NULL) { @@ -3075,7 +3076,8 @@ task_step_handle_result_impl(asyncio_state *state, TaskObj *task, PyObject *resu // // See https://github.com/python/cpython/issues/126138 PyObject *task_cancel_msg = Py_NewRef(task->task_cancel_msg); - r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), task_cancel_msg); + r = PyObject_CallMethodOneArg(result, &_Py_ID(cancel), + task_cancel_msg); Py_DECREF(task_cancel_msg); if (r == NULL) {