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

Integer overflow in _bsddb leads to heap corruption #70131

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

Integer overflow in _bsddb leads to heap corruption #70131

NedWilliamson mannequin opened this issue Dec 25, 2015 · 4 comments
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 25943
Nosy @malemburg, @serhiy-storchaka, @ZackerySpytz
PRs
  • [2.7] bpo-25943: Fix potential heap corruption in bsddb's _db_associateCallback() #8337
  • [2.7] bpo-25943: Check for integer overflow in bsddb's DB_join() #8392
  • Files
  • bsddbpoc.py
  • 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 2018-07-21.13:41:04.117>
    created_at = <Date 2015-12-25.03:53:35.659>
    labels = ['extension-modules', 'type-crash']
    title = 'Integer overflow in _bsddb leads to heap corruption'
    updated_at = <Date 2018-07-22.16:53:59.119>
    user = 'https://bugs.python.org/NedWilliamson'

    bugs.python.org fields:

    activity = <Date 2018-07-22.16:53:59.119>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-07-21.13:41:04.117>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2015-12-25.03:53:35.659>
    creator = 'Ned Williamson'
    dependencies = []
    files = ['41408']
    hgrepos = []
    issue_num = 25943
    keywords = ['patch']
    message_count = 4.0
    messages = ['256974', '322087', '322143', '322154']
    nosy_count = 4.0
    nosy_names = ['lemburg', 'serhiy.storchaka', 'Ned Williamson', 'ZackerySpytz']
    pr_nums = ['8337', '8392']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue25943'
    versions = ['Python 2.7']

    @NedWilliamson
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Dec 25, 2015

    In function _db_associateCallback of the _bsddb module, associating two databases with a callback that returns a sufficiently large list will lead to heap corruption due an integer overflow on 32-bit Python.

    From _bsddb.c:

        else if (PyList_Check(result))
        {
            char* data;
            Py_ssize_t size;
            int i, listlen;
            DBT* dbts;
    
            listlen = PyList_Size(result);
    
    1.      dbts = (DBT *)malloc(sizeof(DBT) * listlen); ///sizeof(DBT) == 28 on my system, enough to overflow
    
    2.      for (i=0; i<listlen; i++)
            {
                if (!PyBytes_Check(PyList_GetItem(result, i)))
                {
                    PyErr_SetString(
                       PyExc_TypeError,
    #if (PY_VERSION_HEX < 0x03000000)
    "The list returned by DB->associate callback should be a list of strings.");
    #else
    "The list returned by DB->associate callback should be a list of bytes.");
    #endif
                    PyErr_Print();
                }
    
                PyBytes_AsStringAndSize(
                    PyList_GetItem(result, i),
    3.              &data, &size);
    
                CLEAR_DBT(dbts[i]);
    4.          dbts[i].data = malloc(size);          /* TODO, check this */
    
                if (dbts[i].data)
                {
    5.              memcpy(dbts[i].data, data, size);
                    dbts[i].size = size;
                    dbts[i].ulen = dbts[i].size;
                    dbts[i].flags = DB_DBT_APPMALLOC;  /* DB will free */
                }
                else
                {
                    PyErr_SetString(PyExc_MemoryError,
                        "malloc failed in _db_associateCallback (list)");
                    PyErr_Print();
                }
            }
    
            CLEAR_DBT(*secKey);
    
            secKey->data = dbts;
            secKey->size = listlen;
            secKey->flags = DB_DBT_APPMALLOC | DB_DBT_MULTIPLE;
            retval = 0;
        }
    
    1. The multiplication in this line can overflow, allocating an undersized buffer.
    2. This loop does not suffer from the overflow, so it can corrupt the heap by writing user data (see 3. and 5.).

    This bug is present in Python 2.7.11.

    See the result of running my attached POC script:

    (gdb) r vuln.py
    Starting program: /vagrant/Python-2.7.11/python.exe vuln.py
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
    python.exe: malloc.c:2372: sysmalloc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 *(sizeof(size_t))) - 1)) & ~((2 *(sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long) old_end & pagemask) == 0)' failed.
    
    Program received signal SIGABRT, Aborted.
    0xb7fdd428 in __kernel_vsyscall ()
    (gdb) bt
    #0  0xb7fdd428 in __kernel_vsyscall ()
    #1  0xb7de6607 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
    #2  0xb7de9a33 in __GI_abort () at abort.c:89
    #3  0xb7e2a9dd in __malloc_assert (
        assertion=assertion@entry=0xb7f1e3c0 "(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offs"...,
        file=file@entry=0xb7f19954 "malloc.c", line=line@entry=2372,
        function=function@entry=0xb7f19ce5 <__func__.10915> "sysmalloc") at malloc.c:293
    #4  0xb7e2d5eb in sysmalloc (av=0xb7f62420 <main_arena>, nb=16) at malloc.c:2369
    #5  _int_malloc (av=av@entry=0xb7f62420 <main_arena>, bytes=bytes@entry=1) at malloc.c:3800
    #6  0xb7e2e708 in __GI___libc_malloc (bytes=1) at malloc.c:2891
    #7  0xb7b006b2 in _db_associateCallback (db=0x82a7dd0, priKey=0xbffff228, priData=0xbffff034, secKey=0x8291a80)
        at /vagrant/Python-2.7.11/Modules/_bsddb.c:1531
    ...
    

    We can see that the malloc call on the line marked (4.) fails due to corrupted heap structures.
    Also, running the script outside of GDB leads to a different message because of differences in heap layout:

    vagrant@vagrant-ubuntu-trusty-32:/vagrant/Python-2.7.11$ ./python.exe vuln.py
    *** Error in `python': corrupted double-linked list: 0x099e9858 ***
    Aborted (core dumped)
    

    This vulnerability can be fixed by checking for the overflow before the call to malloc. Also, note that the PyBytes_Check check does not exit the function, but PyBytesAsStringAndSize is called immediately afterwards. I would recommend breaking or continuing if that check fails, although I do think PyBytesAsStringAndSize performs this check as well.

    @NedWilliamson NedWilliamson mannequin added stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump 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
    Copy link
    Member

    New changeset 3252205 by Serhiy Storchaka (Zackery Spytz) in branch '2.7':
    bpo-25943: Fix potential heap corruption in bsddb's _db_associateCallback() (GH-8337)
    3252205

    @ZackerySpytz
    Copy link
    Mannequin

    ZackerySpytz mannequin commented Jul 22, 2018

    Integer overflow can also occur in DB_join().

    @serhiy-storchaka
    Copy link
    Member

    New changeset 041a4ee by Serhiy Storchaka (Zackery Spytz) in branch '2.7':
    bpo-25943: Check for integer overflow in bsddb's DB_join(). (GH-8392)
    041a4ee

    @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