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

Possible integer overflow in PyCode_New() #62495

Closed
vstinner opened this issue Jun 24, 2013 · 12 comments
Closed

Possible integer overflow in PyCode_New() #62495

vstinner opened this issue Jun 24, 2013 · 12 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@vstinner
Copy link
Member

BPO 18295
Nosy @loewis, @vstinner, @tiran, @serhiy-storchaka, @zooba
Files
  • code_ssize_t.patch
  • code_ssize_t_2.patch.patch
  • 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 2018-09-19.23:18:50.544>
    created_at = <Date 2013-06-24.21:19:05.425>
    labels = ['interpreter-core', 'type-crash']
    title = 'Possible integer overflow in PyCode_New()'
    updated_at = <Date 2018-09-19.23:18:50.543>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2018-09-19.23:18:50.543>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-09-19.23:18:50.544>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2013-06-24.21:19:05.425>
    creator = 'vstinner'
    dependencies = []
    files = ['32712', '38116']
    hgrepos = []
    issue_num = 18295
    keywords = ['patch']
    message_count = 12.0
    messages = ['191805', '191806', '191809', '191900', '203443', '224355', '235853', '235896', '236103', '246361', '272886', '321644']
    nosy_count = 6.0
    nosy_names = ['loewis', 'vstinner', 'christian.heimes', 'python-dev', 'serhiy.storchaka', 'steve.dower']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue18295'
    versions = ['Python 2.7', 'Python 3.6']

    @vstinner
    Copy link
    Member Author

    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: bpo-9566.

    @vstinner vstinner added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 24, 2013
    @vstinner
    Copy link
    Member Author

    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.

    @vstinner
    Copy link
    Member Author

    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.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jun 26, 2013

    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.

    @vstinner
    Copy link
    Member Author

    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!

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 30, 2014

    Note this is referenced from bpo-18407.

    @serhiy-storchaka
    Copy link
    Member

    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.

    @serhiy-storchaka serhiy-storchaka added the type-crash A hard crash of the interpreter, possibly with a core dump label Feb 12, 2015
    @zooba
    Copy link
    Member

    zooba commented Feb 13, 2015

    Other than my one query on the review, code_ssize_t_2.patch.patch looks good to me.

    @serhiy-storchaka
    Copy link
    Member

    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

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 6, 2015

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 16, 2016

    New changeset e615718a6455 by Victor Stinner in branch 'default':
    Use Py_ssize_t in _PyEval_EvalCodeWithName()
    https://hg.python.org/cpython/rev/e615718a6455

    @serhiy-storchaka
    Copy link
    Member

    Since 3.7 the number of arguments no longer limited by 255 (see bpo-12844 and bpo-18896).

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants