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 winapi_createprocess #67550

Closed
pkt mannequin opened this issue Feb 1, 2015 · 4 comments
Closed

integer overflow in winapi_createprocess #67550

pkt mannequin opened this issue Feb 1, 2015 · 4 comments
Labels
type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@pkt
Copy link
Mannequin

pkt mannequin commented Feb 1, 2015

BPO 23361

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-02-10.02:00:18.076>
created_at = <Date 2015-02-01.13:51:43.468>
labels = ['type-crash']
title = 'integer overflow in winapi_createprocess'
updated_at = <Date 2015-02-12.07:55:18.509>
user = 'https://bugs.python.org/pkt'

bugs.python.org fields:

activity = <Date 2015-02-12.07:55:18.509>
actor = 'Arfrever'
assignee = 'none'
closed = True
closed_date = <Date 2015-02-10.02:00:18.076>
closer = 'python-dev'
components = []
creation = <Date 2015-02-01.13:51:43.468>
creator = 'pkt'
dependencies = []
files = []
hgrepos = []
issue_num = 23361
keywords = []
message_count = 4.0
messages = ['235168', '235427', '235607', '235657']
nosy_count = 3.0
nosy_names = ['Arfrever', 'python-dev', 'pkt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue23361'
versions = ['Python 3.3', 'Python 3.4', 'Python 3.5']

@pkt
Copy link
Mannequin Author

pkt mannequin commented Feb 1, 2015

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.

@pkt pkt mannequin added the type-crash A hard crash of the interpreter, possibly with a core dump label Feb 1, 2015
@pkt
Copy link
Mannequin Author

pkt mannequin commented Feb 5, 2015

ping

1 similar comment
@pkt
Copy link
Mannequin Author

pkt mannequin commented Feb 9, 2015

ping

@python-dev
Copy link
Mannequin

python-dev mannequin commented Feb 10, 2015

New changeset ab2e79c6cf6b by Benjamin Peterson in branch '3.3':
add overflow checking (closes bpo-23361)
https://hg.python.org/cpython/rev/ab2e79c6cf6b

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

New changeset 76170e33f251 by Benjamin Peterson in branch 'default':
merge 3.4 (bpo-23361)
https://hg.python.org/cpython/rev/76170e33f251

@python-dev python-dev mannequin closed this as completed Feb 10, 2015
@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
type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

0 participants