mirror of
https://github.com/calesthio/OpenMontage.git
synced 2026-08-24 17:10:27 +08:00
fix(math_animate): harden scan against no-import builtins/reflection bypass
The scan only flagged dangerous builtins as direct call targets (ast.Name func)
and dunders as attribute access, so it missed indirection like
`__builtins__['open']('.env').read()` and `getattr(o, '__class__')` — the
default path still executed secret-reading code.
Block dangerous identifiers wherever they appear as a bare name (open, eval,
exec, compile, __import__, __builtins__, getattr/setattr/delattr, globals/
locals/vars) rather than only as a call target, and extend the blocked dunder
set (__class__, __dict__, __getattribute__, __reduce__, ...). This closes the
reported no-import bypass while genuine math scenes still pass.
Still defense-in-depth, not a full sandbox; the allow_unsafe_code opt-out and
explicit code-execution contract remain. A subprocess-level sandbox is the
right follow-up for complete isolation.
Refs #219
This commit is contained in:
@@ -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():
|
||||
|
||||
@@ -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}'")
|
||||
|
||||
Reference in New Issue
Block a user