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

Type confusion in partial_setstate and partial_call leads to memory corruption #70133

Closed
NedWilliamson mannequin opened this issue Dec 25, 2015 · 6 comments
Closed

Type confusion in partial_setstate and partial_call leads to memory corruption #70133

NedWilliamson mannequin opened this issue Dec 25, 2015 · 6 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@NedWilliamson
Copy link
Mannequin

NedWilliamson mannequin commented Dec 25, 2015

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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Dec 25, 2015

    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)

    @NedWilliamson NedWilliamson mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels Dec 25, 2015
    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Dec 25, 2015
    @serhiy-storchaka serhiy-storchaka self-assigned this Dec 25, 2015
    @serhiy-storchaka
    Copy link
    Member

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

    @ncoghlan
    Copy link
    Contributor

    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
    Copy link
    Contributor

    This looks correct.

    @serhiy-storchaka
    Copy link
    Member

    Thank you Nick and Raymond for your reviews.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 2, 2016

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants