Reproduction steps:
- v1.3.0 fixed the previously-reported bug that
import subprocessinside a
sandboxed handler bypassed subprocess:false (community fix 33a01f):
install_restrictions() now calls _install_import_guard(ns, slug), which
copies vars(builtins) into a new dict, patches only that copy's
__import__ to route gated names ("os", "subprocess") to the sandbox
proxies already sitting in ns, and assigns the copy to ns["__builtins__"].
- Confirm the fix holds for the case it targets:
ns = {"os": __import__("os"), ...}
module_sandbox.install_restrictions(ns, {"subprocess": False}, "evil/mod")
exec("def h():\n import subprocess\n return subprocess.check_output(['echo','x'])", ns)
ns["h"]() # -> SandboxViolation (correct; this is what 33a01f fixed)
- Bypass via the REAL
builtinsmodule, which the guard never touches (only
"os" and "subprocess" are in the gated-name set; "builtins" is not, so
import builtins falls through to the real importer and returns the
pristine, unpatched module object — not the ns-scoped copy):
exec("def h1():\n import builtins\n real_import = builtins.__import__\n "
"sp = real_import('subprocess')\n return sp.check_output(['echo','x']).decode()", ns)
ns["h1"]()
- Bypass via
importlib, also never gated:
exec("def h2():\n import importlib\n real_sp = importlib.import_module('subprocess')\n "
"return real_sp.check_output(['echo','x']).decode()", ns)
ns["h2"]()
- The same class of bypass defeats the filesystem_writes allowlist, since it
is enforced through the identical os-proxy/import-guard mechanism:
module_sandbox.install_restrictions(ns, {"filesystem_writes": ["<tmpdir>/acme/**"]}, "acme/pay")
ns["os"].remove("<tmpdir>/victim.txt") # -> SandboxViolation (correct)
exec("def h(): import importlib; importlib.import_module('os').remove('<tmpdir>/victim.txt')", ns)
ns["h"]() # removes it anyway
- Run repro_module_sandbox_import_guard_bypass_v130.py against a clean v1.3.0
extraction. It drives the REAL module_sandbox.install_restrictions for
both the subprocess:false case and the filesystem_writes case, with a
control proving the guard blocks the case 33a01f targeted. Output:
CONTROL bare import subprocess -> blocked as expected
BYPASS import builtins; builtins.__import__('subprocess') -> 'ESCAPED_VIA_BUILTINS_MODULE'
BYPASS importlib.import_module('subprocess') -> 'ESCAPED_VIA_IMPORTLIB'
CONTROL ns['os'].remove(outside allowlist) -> blocked as expected
BYPASS importlib.import_module('os').remove(outside allowlist) -> SUCCEEDED
victim still exists on disk? False
Expected:
A module that declared subprocess: false, or a narrow filesystem_writes
allowlist, cannot reach the real, unrestricted subprocess/os module by any
standard import mechanism. 33a01f's own stated goal was to make `subprocess:
false hold "against import subprocess` the same way the network gate holds
against import socket" — the network gate achieves that by monkey-patching
the REAL process-wide socket/urllib module objects, so it is immune to this
class of bypass; the subprocess/os gate instead swaps NAMES inside a
namespace-local proxy dict and a namespace-local __builtins__ copy, which by
construction cannot survive a caller reaching the real modules through any
route that doesn't go through that copy.
Actual:_install_import_guard patches __import__ only inside a fresh dict
(safe_builtins = dict(vars(_builtins))) assigned to ns["__builtins__"], and_guarded_import redirects only names present in ns["__rc_sandbox_gated__"]
— which is populated with exactly {"os", "subprocess"}. Two routes bypass
this cleanly:
(a) import builtins is itself an ungated top-level import, so it falls
through _guarded_import to _real_import(...), which returns the ACTUAL
builtins module (not ns["__builtins__"]'s copy). Its __import__
attribute was never modified — the fix only ever mutated a COPY — so
builtins.__import__('subprocess') returns the real subprocess module.
(b) import importlib is likewise ungated, returns the real importlib
module, and importlib.import_module('subprocess') (or 'os') returns
the real module directly, with no involvement of __import__ at all.
Both routes hand the handler the pristine, unpatched subprocess/os modules,
so subprocess.check_output(...) spawns for real despite subprocess: false,
and os.remove(...) deletes outside the filesystem_writes allowlist despite
the gate correctly blocking the identical call through ns["os"]. This is not
the previously-reported bug (33a01f fixed import subprocess/import os
specifically) — it is a new, narrower bypass of the fix itself, reachable with
two extra lines of handler code and no special privileges beyond what any
installed module already has (module trust default is "any", so a self-signed
bundle installs and runs this).
Scope: the network gate (socket/urllib.request) is NOT affected by this
class — it patches the real module objects process-wide, soimportlib.import_module('socket') returns the already-patched module. The
gap is specific to the subprocess/os mechanism, which was rebuilt in v1.3.0
around namespace/builtins substitution instead of process-wide patching.
Suggested fix:
Don't rely on substitution surviving every route to the real module. Either:
1. Patch the spawn/mutator primitives process-wide, gated by the
_ACTIVE_SANDBOX contextvar — exactly the pattern the network gate
already uses for socket/urllib, and exactly what the ORIGINAL
bug's suggested fix proposed (patch subprocess.Popen.__init__ and
os.system/os.popen on the real modules, checking
_ACTIVE_SANDBOX.get() inside the wrapper). This is immune to
importlib/builtins/sys.modules access because there is only one
subprocess.Popen/os.remove object in the process, and it's the
patched one, however a handler reaches it; or
2. If namespace substitution is kept, also gate "builtins" and "importlib"
themselves in _guarded_import — but this is a losing arms race
(sys.modules['subprocess'] directly, ctypes, os.posix_spawn via a
freshly-dlopened libc, etc. all remain open routes to the same
primitive), so (1) is the durable fix.
Apply the same reasoning to the filesystem_writes gate, since it uses the
identical ns["__rc_sandbox_gated__"] / _install_import_guard mechanism for
"os" — the fix in (1) closes both at once.