Skip to content

SystemError when builtins is not a dict + eval #112716

Description

@JelleZijlstra

Bug report

Bug description:

If __builtins__ is not a dict, you can get a SystemError:

>>> import types
>>> exec("import builtins; builtins.print(3)", {"__builtins__": types.MappingProxyType({})})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    exec("import builtins; builtins.print(3)", {"__builtins__": types.MappingProxyType({})})
  File "<string>", line 1, in <module>
SystemError: Objects/dictobject.c:1761: bad argument to internal function

Originally found this while playing with https://oskaerik.github.io/theevalgame/

CPython versions tested on:

3.11, CPython main branch

Operating systems tested on:

macOS

Linked PRs

Activity

  1. added
    3.11only security fixes
    3.12only security fixes
    3.13only security fixes
    on Dec 4, 2023
  2. JelleZijlstra commented on Dec 4, 2023

    @JelleZijlstra
    MemberAuthor

    Simpler repro case:

    >>> exec("import whatever", {"__builtins__": 1})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
        exec("import whatever", {"__builtins__": 1})
      File "<string>", line 1, in <module>
    SystemError: Objects/dictobject.c:1761: bad argument to internal function
    
  3. JelleZijlstra commented on Dec 4, 2023

    @JelleZijlstra
    MemberAuthor

    The original issue is because the import_name function in ceval.c blindly assumes that the builtins is a dict while looking up __import__. There are other code paths that make the same assumption:

    >>> exec("pickle.dumps([].__iter__())", {"__builtins__": 1, "pickle": __import__("pickle")})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
        exec("pickle.dumps([].__iter__())", {"__builtins__": 1, "pickle": __import__("pickle")})
      File "<string>", line 1, in <module>
    SystemError: Objects/dictobject.c:1761: bad argument to internal function
    

    (Pickling a list iterator involves looking up the iter builtin.)

    However, other code (e.g., the implementation of _LOAD_GLOBAL in bytecodes.c) does explicitly support non-dict builtins, so the fact that these code paths don't should be considered a bug.

  4. serhiy-storchaka commented on Dec 4, 2023

    @serhiy-storchaka
    Member

    There are tests (test_exec_globals_dict_subclass, test_exec_globals_error_on_get, test_exec_globals_frozen in test_builtin) that explicitly test non-dict __builtins__. Support of non-dicts was added in bpo-14385/#58593. @vstinner, is this feature used?

  5. vstinner commented on Dec 4, 2023

    @vstinner
    Member

    @vstinner, is this feature used?

    I don't know. But for now, I would prefer to fix bugs. If we decide to remove the feature, I suppose that it should be deprecated first?

  6. JelleZijlstra commented on Dec 4, 2023

    @JelleZijlstra
    MemberAuthor

    Worth noting that the use case that brought me here is real (if a little silly): the game wants to enforce that you don't use any builtins, and it enforces that by setting __builtins__ to a proxy that records access (and delegates lookups to the underlying real builtins). That seems like a reasonable thing to do, so I think we should fix rather than deprecate the feature.

  7. oskaerik commented on Dec 5, 2023

    @oskaerik

    Worth noting that the use case that brought me here is real (if a little silly): the game wants to enforce that you don't use any builtins, and it enforces that by setting __builtins__ to a proxy that records access (and delegates lookups to the underlying real builtins). That seems like a reasonable thing to do, so I think we should fix rather than deprecate the feature.

    Just to expand on this: My first implementation of the proxy inherited from dict (but I changed that so I didn't have to worry about other methods than __getitem__, e.g. pop). But I wouldn't have been surprised if there was a strict requirement to inherit from dict. 😊

  8. JelleZijlstra commented on Dec 5, 2023

    @JelleZijlstra
    MemberAuthor

    Note that support for dict subclasses is also somewhat broken, as the code path used by import (and a few other places) bypasses the subclass's __getitem__:

    >>> class fun(dict):
    ...     def __getitem__(self, key):
    ...             raise TypeError("no way")
    ... 
    >>> import builtins
    >>> b = fun(builtins.__dict__)
    >>> exec("import a", {"__builtins__": b})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
        exec("import a", {"__builtins__": b})
      File "<string>", line 1, in <module>
    ModuleNotFoundError: No module named 'a'
    >>> exec("__import__('a')", {"__builtins__": b})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
        exec("__import__('a')", {"__builtins__": b})
      File "<string>", line 1, in <module>
      File "<stdin>", line 3, in __getitem__
        raise TypeError("no way")
    TypeError: no way
    
  9. serhiy-storchaka commented on Dec 5, 2023

    @serhiy-storchaka
    Member

    It's funny that this feature was originally added when trying to build a sandbox, and now it's used in a game that requires you to escape from a sandbox.

  10. JelleZijlstra commented on Dec 5, 2023

    @JelleZijlstra
    MemberAuthor

    The best features are the ones that turn out to be useful in unexpected places!

  11. added 2 commits that reference this issue on Dec 5, 2023
  12. added 3 commits that reference this issue on Dec 14, 2023
  13. added 2 commits that reference this issue on Dec 14, 2023
  14. added a commit that references this issue on Dec 15, 2023
  15. added a commit that references this issue on Feb 11, 2024
  16. added a commit that references this issue on Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

3.11only security fixes3.12only security fixes3.13only security fixestype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions