Issue 27944: two hotshot module issues (original) (raw)

Here I'll describe two issues in the "hotshot" module which can be found in /Modules/_hotshot.c. Note that this module is for Python 2.7 only.

The issues are (1) an uninitialized variable use and (2) a double free.

Issue 1: uninitialized variable usage in unpack_add_info

In the function unpack_add_info we have this code:

static int
unpack_add_info(LogReaderObject *self)
{
    PyObject *key;
    ...
    err = unpack_string(self, &key);
    if (!err) {
        ...
    }
 finally:
    Py_XDECREF(key);
    Py_XDECREF(value);
    return err;
}

The variable "key" is not initialized to NULL. If the call to unpack_string fails, the code will directly call Py_XDECREF on key which is an uninitialized variable.

To fix this we could initialize "key" to NULL.

Here's a PoC:

import hotshot.log

WHAT_ADD_INFO = 0x13 open("./tmp", "w").write(chr(WHAT_ADD_INFO)) logreader = hotshot.log.LogReader("./tmp")

It segfaults here:

(gdb) run poc_27_1.py

Program received signal SIGSEGV, Segmentation fault. 0x00007ffff696e585 in unpack_add_info (self=0x7ffff6b82098) at /home/xx/Python-2.7.12/Modules/_hotshot.c:370 370 Py_XDECREF(key); (gdb) p *key $3 = {_ob_next = 0x438b480f74fff883, _ob_prev = 0x53894801508d4808, ob_refcnt = 0x9066c35b00b60f08, ob_type = 0x841f0f2e66}

Issue 2: double free (Py_DECREF) in unpack_add_info

This is a separate issue from the one described above, but it exists in the same function, unpack_add_info:

static int
unpack_add_info(LogReaderObject *self)
{
    PyObject *key;
    PyObject *value = NULL;
    int err;

    err = unpack_string(self, &key);
    if (!err) {
        err = unpack_string(self, &value);
        if (err)
            Py_DECREF(key);
        else {
            ...
        }
    }
 finally:
    Py_XDECREF(key);
    Py_XDECREF(value);
    return err;
}

If the second call to unpack_string fails, we call Py_DECREF(key). Then we reach the "finally" block and again call Py_XDECREF(key). So key is getting its refcount dropped twice.

To fix this we could simply remove the "if (err)" clause and turn the "else" into "if (!err)". Then we would only have the single call to Py_XDECREF(key) in the "finally" block.

Here's a PoC:

import hotshot.log

content = "\x13\x20" content += "A"*0x20 open("./tmp", "w").write(content) logreader = hotshot.log.LogReader("./tmp")

When run, python dies with SIGABRT here (because it's a debug build with Py_REF_DEBUG defined; otherwise memory would silently be corrupted):

(gdb) r ./poc_27_2.py Fatal Python error: /home/xx/Python-2.7.12/Modules/_hotshot.c:370 object at 0x7ffff6b7e220 has negative ref count -2604246222170760230

Program received signal SIGABRT, Aborted. 0x00007ffff7143295 in raise () from /usr/lib/libc.so.6