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

chain.__setstate__ Type Confusion #72509

Closed
JohnLeitch mannequin opened this issue Oct 1, 2016 · 5 comments
Closed

chain.__setstate__ Type Confusion #72509

JohnLeitch mannequin opened this issue Oct 1, 2016 · 5 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@JohnLeitch
Copy link
Mannequin

JohnLeitch mannequin commented Oct 1, 2016

BPO 28322
Nosy @rhettinger, @serhiy-storchaka
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • itertools.patch: Patch
  • Py35_itertools.py: Repro file
  • itertools_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-10-02.06:21:44.613>
    created_at = <Date 2016-10-01.06:27:48.227>
    labels = ['3.7', 'library', 'type-crash']
    title = 'chain.__setstate__ Type Confusion'
    updated_at = <Date 2017-03-31.16:36:22.966>
    user = 'https://bugs.python.org/JohnLeitch'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:22.966>
    actor = 'dstufft'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2016-10-02.06:21:44.613>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2016-10-01.06:27:48.227>
    creator = 'JohnLeitch'
    dependencies = []
    files = ['44899', '44900', '44905']
    hgrepos = []
    issue_num = 28322
    keywords = ['patch']
    message_count = 5.0
    messages = ['277799', '277804', '277848', '277855', '277856']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'python-dev', 'serhiy.storchaka', 'JohnLeitch']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue28322'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @JohnLeitch
    Copy link
    Mannequin Author

    JohnLeitch mannequin commented Oct 1, 2016

    Python 3.5.2 suffers from a type confusion vulnerability in the chain.__setstate__ method of the itertools module. The issue exists due to lack of argument validation in the chain_setstate() function:

    static PyObject *
    chain_setstate(chainobject *lz, PyObject *state)
    {
        PyObject *source, *active=NULL;
    if (! PyArg_ParseTuple(state, "O|O", &source, &active))
        return NULL;
    
        Py_INCREF(source);
        Py_XSETREF(lz->source, source);
        Py_XINCREF(active);
        Py_XSETREF(lz->active, active);
        Py_RETURN_NONE;
    }

    After parsing the argument tuple, source and active are set without validating that they are iterator objects. This causes issues elsewhere, where the values are passed PyIter_Next:

    static PyObject *
    chain_next(chainobject *lz)
    {
        PyObject *item;
    if (lz->source == NULL)
        return NULL;                                    /* already stopped */
    
        if (lz->active == NULL) {
            PyObject *iterable = PyIter_Next(lz->source);
            if (iterable == NULL) {
                Py_CLEAR(lz->source);
                return NULL;                                /* no more input sources */
            }
            lz->active = PyObject_GetIter(iterable);
            Py_DECREF(iterable);
            if (lz->active == NULL) {
                Py_CLEAR(lz->source);
                return NULL;                                /* input not iterable */
            }
        }
        item = PyIter_Next(lz->active);
        if (item != NULL)
            return item;
        if (PyErr_Occurred()) {
            if (PyErr_ExceptionMatches(PyExc_StopIteration))
                PyErr_Clear();
            else
                return NULL;                                /* input raised an exception */
        }
        Py_CLEAR(lz->active);
        return chain_next(lz);                      /* recurse and use next active */
    }

    In some cases, this can lead to a DEP access violation. It might be possible to exploit this to achieve code execution.

    (4074.198c): Access violation - code c0000005 (first chance)
    First chance exceptions are reported before any exception handling.
    This exception may be expected and handled.
    eax=00000000 ebx=0132fa10 ecx=5b547028 edx=00000002 esi=0132fa10 edi=5b37b3e0
    eip=00000000 esp=009ef940 ebp=009ef94c iopl=0 nv up ei pl zr na pe nc
    cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010246
    0000000 ?? ???
    0:000> k6
    ChildEBP RetAddr
    WARNING: Frame IP not in any known module. Following frames may be wrong.
    009ef93c 5b329ac0 0x0
    009ef94 5b2cb321 python35!PyIter_Next+0x10 [c:\build\cpython\objects\abstract.c @ 2778]
    009ef960 5b37b42e python35!chain_next+0x21 [c:\build\cpython\modules\itertoolsmodule.c @ 1846]
    009ef970 5b33fedd python35!wrap_next+0x4e [c:\build\cpython\objects\typeobject.c @ 5470]
    009ef990 5b328b9d python35!wrapper_call+0x7d [c:\build\cpython\objects\descrobject.c @ 1195]
    009ef9ac 5b3c463c python35!PyObject_Call+0x6d [c:\build\cpython\objects\abstract.c @ 2167]

    To fix this issue, it is recommended that chain_setstate() be updated to validate its arguments. A proposed patch has been attached.

    static PyObject *
    chain_setstate(chainobject *lz, PyObject *state)
    {
        PyObject *source, *active=NULL;
    if (! PyArg_ParseTuple(state, "O|O", &source, &active))
        return NULL;
    
        if (!PyIter_Check(source) || (active != NULL && !PyIter_Check(active))) {
            PyErr_SetString(PyExc_ValueError, "Arguments must be iterators.");
            return NULL;
        }
    
        Py_INCREF(source);
        Py_XSETREF(lz->source, source);
        Py_XINCREF(active);
        Py_XSETREF(lz->active, active);
        Py_RETURN_NONE;
    }

    @JohnLeitch JohnLeitch mannequin added type-security A security issue stdlib Python modules in the Lib dir labels Oct 1, 2016
    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Oct 1, 2016
    @serhiy-storchaka
    Copy link
    Member

    There is similar issue with itertools.cycle(). And leaking SystemError to user code should be considered as a bug.

    Proposed patch fixes these issues.

    @serhiy-storchaka serhiy-storchaka added type-crash A hard crash of the interpreter, possibly with a core dump and removed type-security A security issue labels Oct 1, 2016
    @rhettinger
    Copy link
    Contributor

    The patch looks reasonable.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 2, 2016

    New changeset 258ebc539b2e by Serhiy Storchaka in branch '3.5':
    Issue bpo-28322: Fixed possible crashes when unpickle itertools objects from
    https://hg.python.org/cpython/rev/258ebc539b2e

    New changeset c4937d066a8e by Serhiy Storchaka in branch '3.6':
    Issue bpo-28322: Fixed possible crashes when unpickle itertools objects from
    https://hg.python.org/cpython/rev/c4937d066a8e

    New changeset cb0755aa9f3d by Serhiy Storchaka in branch 'default':
    Issue bpo-28322: Fixed possible crashes when unpickle itertools objects from
    https://hg.python.org/cpython/rev/cb0755aa9f3d

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution John.

    cycle.__setstate__() was fixed in a4d5ef7fdec3 in 3.6, but the fix was not backported to 3.5. There is no pickle support in 2.7.

    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 2, 2016
    @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
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants