classification
Title: Possible integer overflow in PyCode_New()
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, loewis, python-dev, serhiy.storchaka, steve.dower, vstinner
Priority: normal Keywords: patch

Created on 2013-06-24 21:19 by vstinner, last changed 2018-09-19 23:18 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
code_ssize_t.patch vstinner, 2013-11-19 23:09 review
code_ssize_t_2.patch.patch serhiy.storchaka, 2015-02-12 20:32 review
Messages (12)
msg191805 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-06-24 21:19
On Windows x64, we get the following warning:

..\Objects\codeobject.c(106): warning C4244: '=' : conversion from 'Py_ssize_t' to 'unsigned char', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]

Code:

            unsigned char *cell2arg = NULL;
            Py_ssize_t total_args = argcount + kwonlyargcount +
            ((flags & CO_VARARGS) != 0) + ((flags & CO_VARKEYWORDS) != 0);
            PyObject *cell = PyTuple_GET_ITEM(cellvars, i);
            for (j = 0; j < total_args; j++) {
                PyObject *arg = PyTuple_GET_ITEM(varnames, j);
                if (!PyUnicode_Compare(cell, arg)) {
      ====>         cell2arg[i] = j; <===== HERE
                    used_cell2arg = 1;
                    break;
                }
            }

total_args is not checked for being smaller than 256.

Related issue: #9566.
msg191806 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-06-24 21:23
Similar issue:

..\Objects\funcobject.c(636): warning C4244: 'function' : conversion from 'Py_ssize_t' to 'int', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]
..\Objects\funcobject.c(637): warning C4244: 'function' : conversion from 'Py_ssize_t' to 'int', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]
..\Objects\funcobject.c(637): warning C4244: 'function' : conversion from 'Py_ssize_t' to 'int', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]

Extract of function_call() function:

    result = PyEval_EvalCodeEx(
        PyFunction_GET_CODE(func),
        PyFunction_GET_GLOBALS(func), (PyObject *)NULL,
        &PyTuple_GET_ITEM(arg, 0), PyTuple_GET_SIZE(arg),
        k, nk, d, nd,
        PyFunction_GET_KW_DEFAULTS(func),
        PyFunction_GET_CLOSURE(func));

argcount, kwcount and defcount are int, whereas function_call() pass Py_ssize_t values.

function_call() should check PyTuple_GET_SIZE(arg) <= INT_MAX, nk <= INT_MAX and nd <= INT_MAX.
msg191809 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-06-24 21:31
And another one:

..\Python\ceval.c(4271): warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]
..\Python\ceval.c(4459): warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data [C:\buildbot.python.org\3.x.kloth-win64\build\PCbuild\pythoncore.vcxproj]

First in fast_function(), nd type is int:

    if (argdefs != NULL) {
        d = &PyTuple_GET_ITEM(argdefs, 0);
 ==>    nd = Py_SIZE(argdefs);  <=== HERE
    }
    return PyEval_EvalCodeEx((PyObject*)co, globals,
                             (PyObject *)NULL, (*pp_stack)-n, na,
                             (*pp_stack)-2*nk, nk, d, nd, kwdefs,
                             PyFunction_GET_CLOSURE(func));

Second in ext_do_call(), nstar type is int:

        nstar = PyTuple_GET_SIZE(stararg);

Must check: Py_SIZE(argdefs) <= INT_MAX and PyTuple_GET_SIZE(stararg) <= INT_MAX.
msg191900 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2013-06-26 09:17
I don't think they are actually the *same* issue.

For the limitations wrt. code objects (maximum size of byte code, maximum number of local variables, maximum number of parameters, etc.), I recommend the following thorough procedure:

1. document in a single text file all the limitations
2. check for each one whether an int is sufficient to represent them at runtime.
3. if yes: leave all structure definitions as-is. Local variables might be  changed to size_t where this simplifies the code. Otherwise, Py_SAFE_DOWNCAST should be used where the actual value ought to be valid already. Runtime errors where a value may come from the outside that might be out of range.
4. if not: widen the structures to Py_ssize_t.
msg203443 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-19 23:09
Here is a patch adding Py_SAFE_DOWNCAST(). For update_star_args(), I changed the type instead, because I prefer to avoid Py_SAFE_DOWNCAST() when possible.

Modify PyEval_EvalCodeEx() and PyCode_New() to use Py_ssize_t would be more correct, but it may be slower if Py_ssize_t is larger than int, and I hope that nobody calls functions with more than INT_MAX parameters! It would be completly inefficient!
msg224355 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-30 22:21
Note this is referenced from #18407.
msg235853 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-12 20:32
Many of these overflows can be provoked by specially constructed function, code object or bytecode.

Also I think following examples crash or return wrong result on 64 bit platform:

def f(*args, **kwargs): return len(args), len(kwargs)

f(*([0]*(2**32+1)))
f(**dict.fromkeys(map(hex, range(2**31+1))))

Here is updated patch which handles overflows in non-debug build. It prevent creating Python function with more than 255 default values (in any case compiler and interpreter don't support more than 255 arguments) and raise exception when function is called with too many arguments or too large *args or **kwargs.
msg235896 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-02-13 16:05
Other than my one query on the review, code_ssize_t_2.patch.patch looks good to me.
msg236103 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-16 13:45
It is possible to reproduce original bug without hacking the code object or bytecode:

>>> eval('lambda %s, *args, **kwargs: (lambda:args)' % (', '.join('a%d'%i for i in range(253)),))(*range(256))()
(253, 254, 255)
>>> eval('lambda %s, *args, **kwargs: (lambda:args)' % (', '.join('a%d'%i for i in range(254)),))(*range(256))()
(254, 255)
>>> eval('lambda %s, *args, **kwargs: (lambda:args)' % (', '.join('a%d'%i for i in range(255)),))(*range(256))()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <lambda>
NameError: free variable 'args' referenced before assignment in enclosing scope

>>> eval('lambda %s, *args, **kwargs: (lambda:kwargs)' % (', '.join('a%d'%i for i in range(253)),))(*range(256))()
{}
>>> eval('lambda %s, *args, **kwargs: (lambda:kwargs)' % (', '.join('a%d'%i for i in range(254)),))(*range(256))()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <lambda>
NameError: free variable 'kwargs' referenced before assignment in enclosing scope
>>> eval('lambda %s, *args, **kwargs: (lambda:kwargs)' % (', '.join('a%d'%i for i in range(255)),))(*range(256))()
0
msg246361 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2015-07-06 13:51
Could we try and get this closed please, as I'm always a little concerned that a code change causes a genuine warning that should be actioned, but it gets masked by all the others.
msg272886 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-16 21:48
New changeset e615718a6455 by Victor Stinner in branch 'default':
Use Py_ssize_t in _PyEval_EvalCodeWithName()
https://hg.python.org/cpython/rev/e615718a6455
msg321644 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-14 07:25
Since 3.7 the number of arguments no longer limited by 255 (see issue12844 and issue18896).
History
Date User Action Args
2018-09-19 23:18:50vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-07-14 07:25:24serhiy.storchakasetmessages: + msg321644
versions: + Python 2.7, Python 3.6, - Python 3.4, Python 3.5
2016-08-17 23:03:16BreamoreBoysetnosy: - BreamoreBoy
2016-08-16 21:48:59python-devsetnosy: + python-dev
messages: + msg272886
2015-07-06 13:51:22BreamoreBoysetmessages: + msg246361
2015-02-16 13:45:59serhiy.storchakasetmessages: + msg236103
2015-02-13 16:05:05steve.dowersetnosy: + steve.dower
messages: + msg235896
2015-02-12 20:32:02serhiy.storchakasetfiles: + code_ssize_t_2.patch.patch
type: crash
messages: + msg235853

stage: patch review
2014-07-30 22:21:00BreamoreBoysetnosy: + BreamoreBoy

messages: + msg224355
versions: + Python 3.5
2013-11-19 23:09:44vstinnersetfiles: + code_ssize_t.patch

nosy: + christian.heimes, serhiy.storchaka
messages: + msg203443

keywords: + patch
2013-06-26 09:17:22loewissetnosy: + loewis
messages: + msg191900
2013-06-24 21:31:06vstinnersetmessages: + msg191809
2013-06-24 21:23:21vstinnersetmessages: + msg191806
2013-06-24 21:19:05vstinnercreate