This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: two hotshot module issues
Type: Stage: resolved
Components: Extension Modules Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: python-dev, tehybel
Priority: normal Keywords:

Created on 2016-09-02 21:21 by tehybel, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Messages (2)
msg274274 - (view) Author: tehybel (tehybel) Date: 2016-09-02 21:21
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
msg274426 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-05 19:45
New changeset aefdfa167f4b by Benjamin Peterson in branch '2.7':
rewrite unpack_add_info, so it has less memory corruption bugs (closes #27944)
https://hg.python.org/cpython/rev/aefdfa167f4b
History
Date User Action Args
2022-04-11 14:58:35adminsetgithub: 72131
2016-09-05 19:45:04python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg274426

resolution: fixed
stage: resolved
2016-09-02 21:21:07tehybelcreate