Type confusion in partial_setstate and partial_call leads to memory corruption

[NedWilliamson]

BPO	25945
Nosy	@rhettinger, @ncoghlan, @serhiy-storchaka
Files	partialpoc2.py partial_setstate.patch

^{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:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-02-02.16:59:49.710>
created_at = <Date 2015-12-25.04:16:41.911>
labels = ['extension-modules', 'type-crash']
title = 'Type confusion in partial_setstate and partial_call leads to memory corruption'
updated_at = <Date 2016-02-02.16:59:49.709>
user = 'https://bugs.python.org/NedWilliamson'

bugs.python.org fields:

activity = <Date 2016-02-02.16:59:49.709>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-02-02.16:59:49.710>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2015-12-25.04:16:41.911>
creator = 'Ned Williamson'
dependencies = []
files = ['41410', '41417']
hgrepos = []
issue_num = 25945
keywords = ['patch']
message_count = 6.0
messages = ['256976', '256984', '258196', '259360', '259398', '259401']
nosy_count = 5.0
nosy_names = ['rhettinger', 'ncoghlan', 'python-dev', 'serhiy.storchaka', 'Ned Williamson']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue25945'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[NedWilliamson]

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

partial_setstate performs no checks on the objects
it is passed as an argument.

static PyObject *
partial_call(partialobject *pto, PyObject *args, PyObject *kw)
{
    PyObject *ret;
    PyObject *argappl = NULL, *kwappl = NULL;

assert (PyCallable_Check(pto->fn));
assert (PyTuple_Check(pto->args)); //assume pto->args is a tuple
                                   //assertion not present in release build
assert (pto->kw == Py_None  ||  PyDict_Check(pto->kw));

    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 _PyEval_EvalCodeWithName does not validate the type of PyObject **args either (I assume this is a fair assumption for _PyEval_EvalCodeWithName, and the bug simply lies in the unsafe partial code.

vagrant@vagrant-ubuntu-wily-64:/vagrant/Python-3.5.1$ gdb -q ./python.exe
...
(gdb) r partialpoc2.py
Starting program: /vagrant/Python-3.5.1/python.exe partialpoc2.py
...
Program received signal SIGSEGV, Segmentation fault.
_PyEval_EvalCodeWithName (_co=0x7ffff7045ae0, globals=<optimized out>, locals=locals@entry=0x0, args=args@entry=0x7ffff6fbc520, argcount=1280, kws=kws@entry=0x0, kwcount=0, defs=0x0, defcount=0,
kwdefs=0x0, closure=0x0, name=0x0, qualname=0x0) at Python/ceval.c:3793
3793 Py_INCREF(x);
(gdb) i r
rax 0x9b4b68 10177384
rbx 0x7ffff6fbc520 140737337083168
rcx 0x1 1
rdx 0x2 2
rsi 0x500 1280
rdi 0x0 0
rbp 0x0 0x0
rsp 0x7fffffffdb30 0x7fffffffdb30
r8 0x500 1280
r9 0x0 0
r10 0x7ffff74a6c58 140737342237784
r11 0x9b4b40 10177344
r12 0x0 0
r13 0x0 0
r14 0x7ffff6fb91e0 140737337070048
r15 0x7ffff7e1a048 140737352147016
rip 0x4fc771 0x4fc771 <_PyEval_EvalCodeWithName+961>
eflags 0x10202 [ IF RF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
(gdb) x/3i $pc
=> 0x4fc771 <_PyEval_EvalCodeWithName+961>: addq $0x1,(%rsi)
0x4fc775 <_PyEval_EvalCodeWithName+965>: cmp %edx,%r8d
0x4fc778 <_PyEval_EvalCodeWithName+968>: mov %rsi,0x18(%rax,%rcx,8)

[serhiy-storchaka]

There are other bugs in partial() that lead to crash, leak, or invalid behavior. Proposed patch fixes these bugs.

[ncoghlan]

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.

[rhettinger]

This looks correct.

[serhiy-storchaka]

Thank you Nick and Raymond for your reviews.

[python-dev]

New changeset 542b5744ddc3 by Serhiy Storchaka in branch '3.5':
Issue bpo-25945: Fixed bugs in functools.partial.
https://hg.python.org/cpython/rev/542b5744ddc3

New changeset 33109176538d by Serhiy Storchaka in branch 'default':
Issue bpo-25945: Fixed bugs in functools.partial.
https://hg.python.org/cpython/rev/33109176538d

New changeset 628ce2975e29 by Serhiy Storchaka in branch '2.7':
Issue bpo-25945: Fixed bugs in functools.partial.
https://hg.python.org/cpython/rev/628ce2975e29