Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-125789: fix side-effects inasyncio callback scheduling methods#125833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
Show file tree
Hide file tree
Changes from1 commit
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
NextNext commit
mitigate side-effects in futures scheduling
  • Loading branch information
@picnixz
picnixz committedOct 22, 2024
commitbe7cac9df76ee13729799088257fe66f9868b119
50 changes: 50 additions & 0 deletionsLib/test/test_asyncio/test_futures.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -929,6 +929,47 @@ def __eq__(self, other):

fut.remove_done_callback(evil())

def test_schedule_callbacks_list_mutation_3(self):
raise NotImplemented

def _test_schedule_callbacks_list_mutation_3(self, exc_type, exc_text=None):
# see https://github.com/python/cpython/issues/125789 for details
fut = self._new_future()

class evil:
def __eq__(self, other):
global mem
mem = other
return False

cb_pad = lambda: ...
fut.add_done_callback(cb_pad) # sets fut->fut_callback0
fut.add_done_callback(evil()) # sets first item in fut->fut_callbacks
# Consume fut->fut_callback0 callback but checks the remaining callbacks,
# thereby invoking evil.__eq__().
fut.remove_done_callback(cb_pad)
self.assertIs(mem, cb_pad)

fake = (
(0).to_bytes(8, 'little') +
id(bytearray).to_bytes(8, 'little') +
(2 ** 63 - 1).to_bytes(8, 'little') +
(0).to_bytes(24, 'little')
)

i2f = lambda num: 5e-324 * num
fut._callbacks[0] = complex(0, i2f(id(fake) + bytes.__basicsize__ - 1))

# We want to call once again evil.__eq__() to set 'mem' to our
# malicious bytearray. However, since we manually modified the
# callbacks list, we will not be able to by-pass the checks.
if exc_text is None:
self.assertRaises(exc_type, fut.remove_done_callback, evil())
else:
self.assertRaisesRegex(exc_type, exc_text, fut.remove_done_callback, evil())

self.assertIs(mem, cb_pad)


@unittest.skipUnless(hasattr(futures, '_CFuture'),
'requires the C _asyncio module')
Expand All@@ -938,6 +979,9 @@ class CFutureDoneCallbackTests(BaseFutureDoneCallbackTests,
def _new_future(self):
return futures._CFuture(loop=self.loop)

def test_schedule_callbacks_list_mutation_3(self):
super()._test_schedule_callbacks_list_mutation_3(RuntimeError, 'corrupted')


@unittest.skipUnless(hasattr(futures, '_CFuture'),
'requires the C _asyncio module')
Expand All@@ -949,13 +993,19 @@ class CSubFuture(futures._CFuture):
pass
return CSubFuture(loop=self.loop)

def test_schedule_callbacks_list_mutation_3(self):
super()._test_schedule_callbacks_list_mutation_3(RuntimeError, 'corrupted')


class PyFutureDoneCallbackTests(BaseFutureDoneCallbackTests,
test_utils.TestCase):

def _new_future(self):
return futures._PyFuture(loop=self.loop)

def test_schedule_callbacks_list_mutation_3(self):
super()._test_schedule_callbacks_list_mutation_3(TypeError)


class BaseFutureInheritanceTests:

Expand Down
70 changes: 55 additions & 15 deletionsModules/_asynciomodule.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -434,15 +434,33 @@ future_schedule_callbacks(asyncio_state *state, FutureObj *fut)
return 0;
}

if (!PyList_Check(fut->fut_callbacks)) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
return -1;
}

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++) {
// Beware: 'call_soon' below may change fut_callbacks or its items
// (see https://github.com/python/cpython/issues/125789 for details).
for (i = 0; fut->fut_callbacks != NULL; i++) {
if (!PyList_Check(fut->fut_callbacks)) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
return -1;
}
if (i >= PyList_GET_SIZE(fut->fut_callbacks)) {
break; // done
}
PyObject *cb_tup = PyList_GET_ITEM(fut->fut_callbacks, i);
if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 2) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
return -1;
}
PyObject *cb = PyTuple_GET_ITEM(cb_tup, 0);
PyObject *ctx = PyTuple_GET_ITEM(cb_tup, 1);

Expand DownExpand Up@@ -1033,6 +1051,11 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
return PyLong_FromSsize_t(cleared_callback0);
}

if (!PyList_Check(self->fut_callbacks)) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
return NULL;
}

len = PyList_GET_SIZE(self->fut_callbacks);
if (len == 0) {
Py_CLEAR(self->fut_callbacks);
Expand All@@ -1041,8 +1064,15 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,

if (len == 1) {
PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, 0);
int cmp = PyObject_RichCompareBool(
PyTuple_GET_ITEM(cb_tup, 0), fn, Py_EQ);
// Beware: PyObject_RichCompareBool below may change fut_callbacks or
// its items (see https://github.com/python/cpython/issues/97592 and
// https://github.com/python/cpython/issues/125789 for details).
if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
return NULL;
}
PyObject *cb = PyTuple_GET_ITEM(cb_tup, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A little nitpicky, but the incref should be oncb notcb_tup. We don't necessarily care if the tuple gets deleted in PyObject_RichCompareBool, we care ifcb gets deleted. This technically works because I don't think there's a way to deletecb at this point if the tuple can't be deleted, but it's still worth noting.

int cmp = PyObject_RichCompareBool(cb, fn, Py_EQ);
if (cmp == -1) {
return NULL;
}
Expand All@@ -1060,24 +1090,34 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
return NULL;
}

// 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);
ret = PyObject_RichCompareBool(PyTuple_GET_ITEM(item, 0), fn, Py_EQ);
// Beware: PyObject_RichCompareBool below may change fut_callbacks or
// its items (see https://github.com/python/cpython/issues/97592 and
// https://github.com/python/cpython/issues/125789 for details).
for (i = 0; self->fut_callbacks != NULL; i++) {
if (!PyList_Check(self->fut_callbacks)) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
goto fail;
}
if (i >= PyList_GET_SIZE(self->fut_callbacks)) {
break; // done
}
PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, i);
if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
goto fail;
}
Py_INCREF(cb_tup);
PyObject *cb = PyTuple_GET_ITEM(cb_tup, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Same thing here, I don't think it's possible to deletecb once the tuple has been incref'd, but if it's somehow possible to replace the first item in the tuple thencb will be freed possibly causing a UAF.

int ret = PyObject_RichCompareBool(cb, fn, Py_EQ);
if (ret == 0) {
if (j < len) {
PyList_SET_ITEM(newlist, j,item);
PyList_SET_ITEM(newlist, j,cb_tup);
j++;
continue;
}
ret = PyList_Append(newlist,item);
ret = PyList_Append(newlist,cb_tup);
}
Py_DECREF(item);
Py_DECREF(cb_tup);
if (ret < 0) {
goto fail;
}
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp