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
Type confusion in partial_setstate and partial_call leads to memory corruption #70133
Comments
static PyObject *
partial_setstate(partialobject *pto, PyObject *state)
{
PyObject *fn, *fnargs, *kw, *dict;
if (!PyArg_ParseTuple(state, "OOOO",
&fn, &fnargs, &kw, &dict))
return NULL;
Py_XDECREF(pto->fn);
Py_XDECREF(pto->args);
Py_XDECREF(pto->kw);
Py_XDECREF(pto->dict);
pto->fn = fn;
pto->args = fnargs; //we control pto->args here
static PyObject *
partial_call(partialobject *pto, PyObject *args, PyObject *kw)
{
PyObject *ret;
PyObject *argappl = NULL, *kwappl = NULL;
if (PyTuple_GET_SIZE(pto->args) == 0) {
argappl = args;
Py_INCREF(args);
} else if (PyTuple_GET_SIZE(args) == 0) {
argappl = pto->args; //partial function called with no arguments
Py_INCREF(pto->args);
} else {
argappl = PySequence_Concat(pto->args, args);
if (argappl == NULL)
return NULL;
}
if (pto->kw == Py_None) {
kwappl = kw;
Py_XINCREF(kw);
} else {
kwappl = PyDict_Copy(pto->kw);
if (kwappl == NULL) {
Py_DECREF(argappl);
return NULL;
}
if (kw != NULL) {
if (PyDict_Merge(kwappl, kw, 1) != 0) {
Py_DECREF(argappl);
Py_DECREF(kwappl);
return NULL;
}
}
}
ret = PyObject_Call(pto->fn, argappl, kwappl); //pto->fn called with non-tuple argappl We can see that in the provided POC there is an increment on a user-controlled address (in this case, the literal refcount of a given "argument" is interpreted as a pointer), as vagrant@vagrant-ubuntu-wily-64:/vagrant/Python-3.5.1$ gdb -q ./python.exe |
There are other bugs in partial() that lead to crash, leak, or invalid behavior. Proposed patch fixes these bugs. |
I didn't do a full review of the C code changes, but the new test cases look good to me, and the changes specifically to partial_setstate also look good. |
This looks correct. |
Thank you Nick and Raymond for your reviews. |
New changeset 542b5744ddc3 by Serhiy Storchaka in branch '3.5': New changeset 33109176538d by Serhiy Storchaka in branch 'default': New changeset 628ce2975e29 by Serhiy Storchaka in branch '2.7': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: