Skip to content

Possible integer overflow in PyCode_New() #62495

Closed

Description

@vstinner
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']

    Activity

    1. vstinner commented on Jun 24, 2013

      @vstinner
      MemberAuthor

      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.

    2. vstinner commented on Jun 24, 2013

      @vstinner
      MemberAuthor

      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.

    3. vstinner commented on Jun 24, 2013

      @vstinner
      MemberAuthor

      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.

    4. loewis commented on Jun 26, 2013

      loewismannequin
      Mannequin

      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.
    5. vstinner commented on Nov 19, 2013

      @vstinner
      MemberAuthor

      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!

    6. BreamoreBoy commented on Jul 30, 2014

      BreamoreBoymannequin
      Mannequin

      Note this is referenced from bpo-18407.

    7. serhiy-storchaka commented on Feb 12, 2015

      @serhiy-storchaka
      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.

    8. zooba commented on Feb 13, 2015

      @zooba
      Member

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

    9. serhiy-storchaka commented on Feb 16, 2015

      @serhiy-storchaka
      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
    10. BreamoreBoy commented on Jul 6, 2015

      BreamoreBoymannequin
      Mannequin

      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.

    11. python-dev commented on Aug 16, 2016

      python-devmannequin
      Mannequin

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

    12. serhiy-storchaka commented on Jul 14, 2018

      @serhiy-storchaka
      Member

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

    13. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      interpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions