From 5343e2e993d6fb2cd76bf4413e380ac6cce1f19c Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Sat, 11 Jul 2026 04:27:30 +0000 Subject: [PATCH] Harden sandbox: const-fold loader-table getattr names, block type(instance) / sqlite Connection MRO recovery Close three MRO / dynamic-attribute recovery gaps Codex found on the round-59 branch (all P1). - const-folded loader-table getattr names: the sys.meta_path / sys.modules recognizers accepted only a raw string literal, so getattr(sys, 'meta_' + 'path').pop(0) (or the sys.modules equivalent) removed the sandbox workdir-module vetter / dropped a guarded module before importing an unguarded workdir child that runs os.system / subprocess outside the session workdir. Add _extract_folded_string and use it for the getattr / __getattribute__ / __getattr__ attribute-name checks, so a folded or const-var name is recognized exactly like the literal. A benign read of the finder chain stays allowed. - type().mro(): the whole-MRO recovery guard only recognized io.FileIO / obj.__class__ receivers, so type(io.FileIO('/dev/null','r')).mro() (or a same-name alias of that type(...)) iterated the guarded subclass's MRO to recover the original unguarded _io.FileIO base and read / write outside the workdir. Recognize type() where constructs a guarded file / sqlite instance as a recovery receiver, and resolve a single-assignment alias of it. - sqlite3.Connection MRO base recovery: the exported sqlite3.Connection is the guarded subclass whose MRO still exposes the unguarded _sqlite3.Connection base, so iterating sqlite3.Connection.mro() / .__mro__ recovered it and instantiated it with an absolute path, bypassing connect() and the guarded __init__. Treat the guarded sqlite3.Connection / _sqlite3.Connection / sqlite3.dbapi2.Connection as a recovery receiver so a whole-MRO walk of it is blocked like io.FileIO. The subscripted / popped forms were already caught; this closes the iteration form. Regression coverage: TestRound60Bypasses in tests/test_sandbox_tools.py (folded meta_path / sys.modules pop / clear / __getattribute__ and del-subscript mutations, type(io.FileIO(...)) / type(sqlite3.connect(...)) whole-MRO access and its alias, sqlite3.Connection / dbapi2.Connection MRO walks, plus a round60 benign-allowed set: reading sys.meta_path, a benign sys getattr, int.mro() / type(42).mro(), a plain class access, an in-memory connect, and a plain dict pop). --- studio/backend/core/inference/tools.py | 97 +++++++++++++++++++--- studio/backend/tests/test_sandbox_tools.py | 81 ++++++++++++++++++ 2 files changed, 168 insertions(+), 10 deletions(-) diff --git a/studio/backend/core/inference/tools.py b/studio/backend/core/inference/tools.py index a2cf910bca..473bb81015 100644 --- a/studio/backend/core/inference/tools.py +++ b/studio/backend/core/inference/tools.py @@ -7348,6 +7348,16 @@ def _check_signal_escape_patterns( return node.value return None + def _extract_folded_string(node): + """Like _extract_string_from_node but also const-folds a computed string, so a folded + dynamic-attribute name is recognized: getattr(sys, 'meta_' + 'path') / a const-var + alias resolve to 'meta_path' exactly like the raw literal.""" + _s = _extract_string_from_node(node) + if _s is not None: + return _s + _f = _const_fold(node, _const_env) + return _f if isinstance(_f, str) else None + def _extract_env_scalar(node): """A str constant, a const-folded string (a const-var / concatenation via the module const env, ``P='.:/usr/bin'; ... P``), or a bytes constant / folded bytes decoded to str @@ -8258,7 +8268,7 @@ def _check_signal_escape_patterns( and n.func.id == "getattr" and len(n.args) >= 2 and _ast_name_matches(n.args[0], self.sys_aliases) - and _extract_string_from_node(n.args[1]) == "modules" + and _extract_folded_string(n.args[1]) == "modules" ): return True # object.__getattribute__(sys, 'modules') / type(sys).__getattribute__(sys, @@ -8270,7 +8280,7 @@ def _check_signal_escape_patterns( and n.func.attr in ("__getattribute__", "__getattr__") and len(n.args) >= 2 and _ast_name_matches(n.args[0], self.sys_aliases) - and _extract_string_from_node(n.args[1]) == "modules" + and _extract_folded_string(n.args[1]) == "modules" ): return True return False @@ -8302,7 +8312,7 @@ def _check_signal_escape_patterns( and n.func.id == "getattr" and len(n.args) >= 2 and _ast_name_matches(n.args[0], self.sys_aliases) - and _extract_string_from_node(n.args[1]) == "meta_path" + and _extract_folded_string(n.args[1]) == "meta_path" ): return True if ( @@ -8311,7 +8321,7 @@ def _check_signal_escape_patterns( and n.func.attr in ("__getattribute__", "__getattr__") and len(n.args) >= 2 and _ast_name_matches(n.args[0], self.sys_aliases) - and _extract_string_from_node(n.args[1]) == "meta_path" + and _extract_folded_string(n.args[1]) == "meta_path" ): return True return False @@ -9802,13 +9812,80 @@ def _check_signal_escape_patterns( ) self.generic_visit(node) + def _is_sqlite_module_ref(self, node): + # sqlite3 / _sqlite3 (Name) or sqlite3.dbapi2 (Attribute): the modules that export the + # guarded Connection subclass whose MRO still exposes the unguarded _sqlite3.Connection. + if isinstance(node, ast.Name): + return node.id in ("sqlite3", "_sqlite3") + if isinstance(node, ast.Attribute): + return ( + node.attr == "dbapi2" + and isinstance(node.value, ast.Name) + and node.value.id == "sqlite3" + ) + return False + + def _is_type_of_guarded_instance(self, expr): + # ``type()`` where ```` constructs a guarded file / sqlite instance, so ``type( + # )`` IS the guarded subclass and iterating its MRO recovers the unguarded base: + # type(io.FileIO('x')).mro(), type(sqlite3.connect(':memory:')).mro(). Only a single + # positional construction arg is matched, so type(x) on an opaque value does not. + if not ( + isinstance(expr, ast.Call) + and isinstance(expr.func, ast.Name) + and expr.func.id == "type" + and len(expr.args) == 1 + and not expr.keywords + ): + return False + arg = expr.args[0] + if not isinstance(arg, ast.Call): + return False + f = arg.func + if isinstance(f, ast.Attribute) and f.attr == "FileIO": + return True # io.FileIO(...) / _io.FileIO(...) + if ( + isinstance(f, ast.Attribute) + and f.attr in ("connect", "Connection") + and self._is_sqlite_module_ref(f.value) + ): + return True # sqlite3.connect(...) / sqlite3.Connection(...) + return False + + def _is_fileclass_recovery_direct(self, expr): + # io.FileIO / _io.FileIO (.FileIO) or a file object's type via .__class__ + # (open.__class__, f.__class__). + if isinstance(expr, ast.Attribute) and expr.attr in ("FileIO", "__class__"): + return True + # The guarded sqlite3.Connection / _sqlite3.Connection / sqlite3.dbapi2.Connection + # subclass, whose MRO still exposes the unguarded _sqlite3.Connection base. + if ( + isinstance(expr, ast.Attribute) + and expr.attr == "Connection" + and self._is_sqlite_module_ref(expr.value) + ): + return True + # type() is that same guarded subclass. + if self._is_type_of_guarded_instance(expr): + return True + return False + def _is_fileclass_recovery_expr(self, expr): - """True when ``expr`` denotes a file/IO class whose MRO walk recovers an UNGUARDED - file primitive: the guarded ``io.FileIO`` / ``_io.FileIO`` (``.FileIO`` attribute) - or the type of a file object reached through ``.__class__`` (``open.__class__``, - ``f.__class__``). Ordinary class receivers (``int``, ``cls``, ``type('X', (), {})``) - are plain Names / Calls and do not match, so benign MRO introspection stays allowed.""" - return isinstance(expr, ast.Attribute) and expr.attr in ("FileIO", "__class__") + """True when ``expr`` denotes a class whose MRO walk recovers an UNGUARDED primitive: + the guarded ``io.FileIO`` / ``_io.FileIO`` (``.FileIO`` attribute), the type of a file + object via ``.__class__`` (``open.__class__``, ``f.__class__``), the guarded + ``sqlite3.Connection`` subclass (whose base is the unguarded ``_sqlite3.Connection``), + or ``type()``. A single-assignment alias of any of these + (``t = type(io.FileIO('x')); t.mro()``) resolves through the scope index. Ordinary class + receivers (``int``, ``cls``, ``type(42)``, ``type('X', (), {})``) do not match, so benign + MRO introspection stays allowed.""" + if self._is_fileclass_recovery_direct(expr): + return True + if _analyzer_on and isinstance(expr, ast.Name): + rhs = _scope_idx.resolve(expr.id, expr, "rhsnode") + if rhs is not None and self._is_fileclass_recovery_direct(rhs): + return True + return False def _is_unbound_mro_gadget(self, node): """True when ``node`` is an UNBOUND MRO / getattribute call that recovers a file diff --git a/studio/backend/tests/test_sandbox_tools.py b/studio/backend/tests/test_sandbox_tools.py index 3f649e7874..7a4ed19ec1 100644 --- a/studio/backend/tests/test_sandbox_tools.py +++ b/studio/backend/tests/test_sandbox_tools.py @@ -5850,3 +5850,84 @@ class TestRound59Bypasses: ) def test_round59_benign_allowed(self, code): _ok(code) + + +class TestRound60Bypasses: + # Loader-table / import-finder gadget names written as a const-folded string + # (getattr(sys, 'meta_' + 'path')) must be recognized like the raw literal, so a folded + # getattr cannot drop the sandbox workdir-module vetter (meta_path) or a guarded module + # (modules) before importing an unguarded workdir child. + @pytest.mark.parametrize( + "code", + [ + "import sys\ngetattr(sys, 'meta_' + 'path').pop(0)", + "import sys\ngetattr(sys, 'meta_' + 'path').clear()", + "import sys\nobject.__getattribute__(sys, 'meta_' + 'path').pop(0)", + ], + ) + def test_folded_meta_path_mutation_blocked(self, code): + _blocked(code, expect_phrase = "import finder chain") + + def test_folded_sys_modules_mutation_blocked(self): + _blocked( + "import sys\ngetattr(sys, 'mod' + 'ules').pop('os')", + expect_phrase = "loader table", + ) + + # del / assign of a folded loader-table subscript reaches the shared del/assign handler that + # blocks with a generic "drop a guarded module" message; the point is the folded form is caught. + @pytest.mark.parametrize( + "code", + [ + "import sys\ndel getattr(sys, 'meta_' + 'path')[0]", + "import sys\ndel getattr(sys, 'mod' + 'ules')['os']", + ], + ) + def test_folded_loader_subscript_del_blocked(self, code): + _blocked(code, expect_phrase = "(del / assign)") + + # type().mro() / .__mro__ iterates a guarded subclass whose MRO + # exposes the original unguarded C base; block the whole-MRO access (and a same-name alias of + # the type(...) result), not just the subscripted / popped forms. + @pytest.mark.parametrize( + "code", + [ + "import io\ntype(io.FileIO('/dev/null', 'r')).mro()", + "import io\nt = type(io.FileIO('/dev/null', 'r'))\nt.mro()", + "import io\nfor c in type(io.FileIO('/dev/null', 'r')).__mro__:\n pass", + "import sqlite3\ntype(sqlite3.connect(':memory:')).mro()", + ], + ) + def test_type_of_instance_mro_blocked(self, code): + _blocked(code, expect_phrase = "unguarded base") + + # The guarded sqlite3.Connection subclass still exposes the unguarded _sqlite3.Connection base + # through its MRO, so a whole-MRO walk of sqlite3.Connection is a recovery gadget too. + @pytest.mark.parametrize( + "code", + [ + "import sqlite3\nsqlite3.Connection.mro()", + "import sqlite3\nfor c in sqlite3.Connection.__mro__:\n pass", + "import sqlite3\ngetattr(sqlite3.Connection, '__mro__')", + "import sqlite3\nsqlite3.dbapi2.Connection.mro()", + ], + ) + def test_sqlite_connection_mro_blocked(self, code): + _blocked(code, expect_phrase = "unguarded base") + + @pytest.mark.parametrize( + "code", + [ + # Benign round-60 forms stay allowed. + "import sys\nx = sys.meta_path", # reading the finder chain + "import sys\nx = sys.meta_path[0]", # reading one finder + "import sys\ngetattr(sys, 'argv')", # a benign sys getattr + "int.mro()", # ordinary MRO introspection + "type(42).mro()", # type() of a non-guarded instance + "x = [].__class__", # a plain class access + "import sqlite3\nsqlite3.connect(':memory:')", # an in-memory connect + "d = {'a': 1}\nd.pop('a')", # a plain dict pop, not a loader table + ], + ) + def test_round60_benign_allowed(self, code): + _ok(code)