classification
Title: integer overflow in winapi_createprocess
Type: crash Stage: resolved
Components: Versions: Python 3.5, Python 3.3, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, pkt, python-dev
Priority: normal Keywords:

Created on 2015-02-01 13:51 by pkt, last changed 2015-02-12 07:55 by Arfrever. This issue is now closed.

Messages (4)
msg235168 - (view) Author: paul (pkt) Date: 2015-02-01 13:51
winapi_createprocess takes env_mapping dictionary as a parameter, mapping variables to their env. values. Dictionary with pathologically large values will cause an integer overflow during computation of total space required to store all key-value pairs


File: Modules\_winapi.c

static PyObject*
getenvironment(PyObject* environment)
{
    Py_ssize_t i, envsize, totalsize;
    ...
    envsize = PyMapping_Length(environment);

    keys = PyMapping_Keys(environment);
    values = PyMapping_Values(environment);
    if (!keys || !values)
        goto error;

    totalsize = 1; /* trailing null character */
    for (i = 0; i < envsize; i++) {
        PyObject* key = PyList_GET_ITEM(keys, i);
        PyObject* value = PyList_GET_ITEM(values, i);

        if (! PyUnicode_Check(key) || ! PyUnicode_Check(value)) {
            PyErr_SetString(PyExc_TypeError,
                "environment can only contain strings");
            goto error;
        }
        totalsize += PyUnicode_GET_LENGTH(key) + 1;    /* +1 for '=' */
1       totalsize += PyUnicode_GET_LENGTH(value) + 1;  /* +1 for '\0' */
    }

2   buffer = PyMem_Malloc(totalsize * sizeof(Py_UCS4));
    if (! buffer)
        goto error;
    p = buffer;
3   end = buffer + totalsize;

4   for (i = 0; i < envsize; i++) {
        PyObject* key = PyList_GET_ITEM(keys, i);
        PyObject* value = PyList_GET_ITEM(values, i);
X       if (!PyUnicode_AsUCS4(key, p, end - p, 0))
            goto error;
        p += PyUnicode_GET_LENGTH(key);
X       *p++ = '=';
X       if (!PyUnicode_AsUCS4(value, p, end - p, 0))
            goto error;
        p += PyUnicode_GET_LENGTH(value);
X       *p++ = '\0';
    }

1. no overflow checks. We can set totalsize to 2^30, with a crafted dictionary.
2. totalsize*4 == 0, so buffer is 0-bytes long
3. end = buffer+2^30
4. envsize == len(env_mapping). We can make this variable as large as we like. 
X. write past the buffer's end. Note size checks in PyUnicode_AsUCS4 are inefficient, because the size variable (end-p) is very large.
msg235427 - (view) Author: paul (pkt) Date: 2015-02-05 10:41
ping
msg235607 - (view) Author: paul (pkt) Date: 2015-02-09 13:21
ping
msg235657 - (view) Author: Roundup Robot (python-dev) Date: 2015-02-10 02:00
New changeset ab2e79c6cf6b by Benjamin Peterson in branch '3.3':
add overflow checking (closes #23361)
https://hg.python.org/cpython/rev/ab2e79c6cf6b

New changeset b82cc9180a78 by Benjamin Peterson in branch '3.4':
merge 3.3 (#23361)
https://hg.python.org/cpython/rev/b82cc9180a78

New changeset 76170e33f251 by Benjamin Peterson in branch 'default':
merge 3.4 (#23361)
https://hg.python.org/cpython/rev/76170e33f251
History
Date User Action Args
2015-02-12 07:55:18Arfreversetversions: + Python 3.3, Python 3.5
2015-02-10 02:00:18python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg235657

resolution: fixed
stage: resolved
2015-02-09 13:21:25pktsetmessages: + msg235607
2015-02-05 10:41:09pktsetmessages: + msg235427
2015-02-01 21:16:48Arfreversetnosy: + Arfrever
2015-02-01 13:51:43pktcreate