Repository navigation
SystemError when builtins is not a dict + eval #112716
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
on Dec 4, 2023 - added3.11only security fixesonly security fixes3.12only security fixesonly security fixes3.13only security fixesonly security fixes
on Dec 4, 2023 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 functionReacted by Serhiy StorchakaThe original issue is because the
import_namefunction inceval.cblindly 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
iterbuiltin.)However, other code (e.g., the implementation of
_LOAD_GLOBALin bytecodes.c) does explicitly support non-dict builtins, so the fact that these code paths don't should be considered a bug.@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 fromdict. 😊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 wayIt'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.
Reacted by Alex Waygood, Jelle Zijlstra, Kirill Podoprigora, Oskar Eriksson, Zsolt Dollenstein, Muzuwi, Itamar Oren, Oleksandr Zinkevych, Satyendra Singh, yakimka and 3 moreReacted by Victor Stinner, Art Ivanov and rewhileThe best features are the ones that turn out to be useful in unexpected places!
- added 3 commits that reference this issue
on Dec 14, 2023
Bug report
Bug description:
If
__builtins__is not a dict, you can get a SystemError: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