SystemError when builtins is not a dict + eval

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

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

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.

@vstinner

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?

@vstinner

@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?

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.

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. 😊

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

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.

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

pythongh-112716: SystemError when __builtins__ is not a dict

pythongh-112716: Fix SystemError when __builtins__ is not a dict

gh-112716: Fix SystemError when __builtins__ is not a dict (GH-112770)

[3.12] pythongh-112716: Fix SystemError when __builtins__ is not a di…

[3.11] pythongh-112716: Fix SystemError when __builtins__ is not a di…

[3.12] gh-112716: Fix SystemError when __builtins__ is not a dict (GH…

[3.11] gh-112716: Fix SystemError when __builtins__ is not a dict (GH…

pythongh-112716: Fix SystemError when __builtins__ is not a dict (pyt…

Navigation Menu

Repository navigation

SystemError when builtins is not a dict + eval #112716

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

SystemError when builtins is not a dict + eval #112716

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

JelleZijlstra commented on Dec 4, 2023

JelleZijlstra commented on Dec 4, 2023

serhiy-storchaka commented on Dec 4, 2023

vstinner commented on Dec 4, 2023

JelleZijlstra commented on Dec 4, 2023

oskaerik commented on Dec 5, 2023

JelleZijlstra commented on Dec 5, 2023

serhiy-storchaka commented on Dec 5, 2023

JelleZijlstra commented on Dec 5, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions