diff --git a/tests/tools/test_math_animate_safety.py b/tests/tools/test_math_animate_safety.py index dafc487d..a83ced7f 100644 --- a/tests/tools/test_math_animate_safety.py +++ b/tests/tools/test_math_animate_safety.py @@ -54,7 +54,45 @@ def test_blocks_dangerous_calls(call): " def construct(self):\n" f" {call}('x')\n" ) - assert f"call to '{call}()'" in MathAnimate._scan_scene_code(code) + assert f"use of '{call}'" in MathAnimate._scan_scene_code(code) + + +def test_blocks_no_import_builtins_secret_read(): + # Regression for the reported bypass: no dangerous import, secret read via + # __builtins__ indexing. The whole expression roots on the bare __builtins__ + # name (the 'open' inside [] is a string literal), so blocking that name + # blocks the payload. + code = ( + "from manim import *\n" + "class S(Scene):\n" + " def construct(self):\n" + " __builtins__['open']('.env').read()\n" + ) + assert "use of '__builtins__'" in MathAnimate._scan_scene_code(code) + + +def test_blocks_getattr_reflection_bypass(): + # getattr-based attribute reflection is a classic denylist evasion; blocking + # the getattr name removes the primitive. + code = ( + "from manim import *\n" + "class S(Scene):\n" + " def construct(self):\n" + " cls = getattr(object(), '__class__')\n" + ) + assert "use of 'getattr'" in MathAnimate._scan_scene_code(code) + + +def test_blocks_aliased_dangerous_builtin(): + # Binding a blocked builtin to another name must still trip on the name use. + code = ( + "from manim import *\n" + "class S(Scene):\n" + " def construct(self):\n" + " f = open\n" + " f('.env')\n" + ) + assert "use of 'open'" in MathAnimate._scan_scene_code(code) def test_blocks_sandbox_escape_dunders(): diff --git a/tools/graphics/math_animate.py b/tools/graphics/math_animate.py index 1caf808e..63be79b7 100644 --- a/tools/graphics/math_animate.py +++ b/tools/graphics/math_animate.py @@ -45,12 +45,20 @@ _BLOCKED_IMPORTS = frozenset({ "importlib", "builtins", "multiprocessing", "threading", "pty", "glob", "resource", "signal", "tempfile", "webbrowser", "pathlib", }) -_BLOCKED_CALLS = frozenset({ +# Dangerous identifiers blocked wherever they appear as a bare name — not just +# as a direct call. This catches indirection like `__builtins__['open']`, +# `f = open`, or `getattr(x, '__class__')` that a call-target-only or +# attribute-only check would miss. +_BLOCKED_NAMES = frozenset({ "eval", "exec", "compile", "__import__", "open", "input", "breakpoint", + "__builtins__", "__loader__", "globals", "locals", "vars", + "getattr", "setattr", "delattr", }) +# Sandbox-escape / reflection dunders blocked as attribute access. _BLOCKED_ATTRS = frozenset({ - "__globals__", "__builtins__", "__subclasses__", "__bases__", "__mro__", - "__code__", + "__globals__", "__builtins__", "__subclasses__", "__bases__", "__base__", + "__mro__", "__code__", "__class__", "__dict__", "__getattribute__", + "__closure__", "__reduce__", "__reduce_ex__", "__subclasshook__", }) @@ -230,10 +238,11 @@ class MathAnimate(BaseTool): root = (node.module or "").split(".")[0] if root in _BLOCKED_IMPORTS: violations.append(f"from '{node.module}' import ...") - elif isinstance(node, ast.Call): - fn = node.func - if isinstance(fn, ast.Name) and fn.id in _BLOCKED_CALLS: - violations.append(f"call to '{fn.id}()'") + elif isinstance(node, ast.Name): + # Blocks direct calls (eval(...)) and indirection alike: + # `__builtins__['open']`, `f = open`, `getattr(o, '__class__')`. + if node.id in _BLOCKED_NAMES: + violations.append(f"use of '{node.id}'") elif isinstance(node, ast.Attribute): if node.attr in _BLOCKED_ATTRS: violations.append(f"attribute access '.{node.attr}'")