Skip to content

Commit ebc951d

Browse files
committed
Close a real sandbox escape: dir_fd= bypasses the workspace write grant
check_path() resolves a call's path argument against the process's cwd, pinned to the workspace by os.chdir(). Every mutating os function that accepts dir_fd=/src_dir_fd=/dst_dir_fd= resolves the same string against an open directory descriptor instead, and cwd never enters into it — so a tool that legitimately opens a runtime path for reading (site-packages, which has to stay readable so imports work) can get a descriptor for it and then write there via that descriptor while check_path validates a path that has nothing to do with where the write actually lands. Verified end to end: a tool opens site-packages (a legitimate read), gets a dir_fd from it, and calls os.open("evil.pth", O_CREAT|O_WRONLY, dir_fd=that_fd). No SandboxViolation was raised, the file landed in site-packages, and it executed automatically on the next fresh interpreter start in that environment — the exact .pth-drop escape the read/write grant split (and the existing os.readlink guard) exists to close, reopened through a syscall shape the audit-event path check never considered a path argument for at all. os.mkdir, os.remove, and os.rename with dir_fd= reproduce the same hole. Fix: _install_dir_fd_guard() wraps every stdlib function that accepts one of the three dir_fd keyword spellings (chmod, chown, link, mkdir, mkfifo, mknod, open, remove, rename, replace, rmdir, symlink, unlink, utime) and refuses the keyword outright, the same way the existing os.readlink guard already refuses dir_fd for the one function it covers — there is no reliable way to turn a directory descriptor back into the path it names, so there is nothing to check against the grant. Derived from the stdlib's own three keyword spellings rather than a hand-maintained per-function table, so it does not go stale the way the fork_exec arity table already did once. Added test_gate_dot_pth_cannot_be_planted_via_dir_fd (4 parametrized cases: os.open, os.mkdir, os.remove, os.rename), sitting next to the existing plain-path .pth-planting gate test it mirrors. Verified: full suite green on Python 3.12 and 3.13; ruff clean; confirmed normal (non-dir_fd) file operations inside the workspace are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent c3d8001 commit ebc951d

2 files changed

Lines changed: 142 additions & 0 deletions

File tree

grapharc/harness/executor.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,84 @@ def fork_exec(*args: Any, **kwargs: Any) -> Any:
383383
module.fork_exec = fork_exec
384384

385385

386+
#: Every stdlib function that accepts a directory-descriptor keyword uses one
387+
#: of exactly three spellings (`dir_fd`, `src_dir_fd`, `dst_dir_fd`), so
388+
#: `_install_dir_fd_guard` below checks all three by name rather than needing
389+
#: a table of which function takes which — a table would go stale the way the
390+
#: `fork_exec` arity above already did once. `readlink` is excluded: it has
391+
#: its own dedicated guard above, for a different reason (its *result*, not
392+
#: just its argument, has to be resolved and checked).
393+
_DIR_FD_MUTATORS = (
394+
"chmod",
395+
"chown",
396+
"link",
397+
"mkdir",
398+
"mkfifo",
399+
"mknod",
400+
"open",
401+
"remove",
402+
"rename",
403+
"replace",
404+
"rmdir",
405+
"symlink",
406+
"unlink",
407+
"utime",
408+
)
409+
410+
411+
def _install_dir_fd_guard() -> None:
412+
"""Refuse `dir_fd=`/`src_dir_fd=`/`dst_dir_fd=` on every mutating call that
413+
accepts one — a real, verified escape from the workspace grant.
414+
415+
`check_path` resolves a call's path argument against the *process's cwd*,
416+
pinned to the workspace by the `os.chdir` below. A directory-descriptor
417+
keyword makes the underlying syscall resolve that same string against an
418+
open file descriptor instead; cwd never enters into it. A tool legitimately
419+
reading a directory inside the read grant — site-packages, say — gets a
420+
descriptor for it, and `os.open("evil.pth", O_CREAT | O_WRONLY,
421+
dir_fd=that_fd)` then writes there while `check_path` is validating a path
422+
that has nothing to do with where the write actually lands. Confirmed by
423+
running it end to end: the planted file executes on the next interpreter
424+
start in that environment — the exact site-packages `.pth`-drop escape the
425+
read/write grant split exists to close (see the module docstring above),
426+
reopened through a syscall shape the audit-event path check never
427+
considered a path argument for at all.
428+
429+
Like the `os.readlink` guard, this refuses the capability outright rather
430+
than trying to resolve and check it: there is no reliable way to turn a
431+
directory descriptor back into the path it names, so there is nothing to
432+
validate against the grant. It is a wrapper, not an audit hook, and carries
433+
the same documented caveat for the same reason — the original stays
434+
reachable through the closure, so it closes the accident, not a
435+
determined adversary already unwrapping functions from inside.
436+
"""
437+
posix = sys.modules.get("posix") or sys.modules.get("nt")
438+
439+
def make_wrapper(original: Any, name: str) -> Any:
440+
def wrapper(*args: Any, **kwargs: Any) -> Any:
441+
for kw in ("dir_fd", "src_dir_fd", "dst_dir_fd"):
442+
if kwargs.get(kw) is not None:
443+
raise SandboxViolation(
444+
f"os.{name} through a directory descriptor ({kw}) resolves "
445+
"outside the process's cwd and cannot be checked against the "
446+
"workspace grant, so it is refused"
447+
)
448+
return original(*args, **kwargs)
449+
450+
wrapper.__name__ = name
451+
wrapper.__qualname__ = name
452+
return wrapper
453+
454+
for name in _DIR_FD_MUTATORS:
455+
original = getattr(os, name, None)
456+
if original is None: # pragma: no cover - platform-dependent (mkfifo/mknod on Windows)
457+
continue
458+
wrapped = make_wrapper(original, name)
459+
setattr(os, name, wrapped)
460+
if posix is not None and hasattr(posix, name):
461+
setattr(posix, name, wrapped)
462+
463+
386464
def _is_sqlite_uri(database: Any) -> bool:
387465
"""sqlite re-reads a `file:` name itself — percent-decoding it and honouring
388466
an authority and query string — so `realpath` does not name the file that
@@ -531,6 +609,7 @@ def hook(event: str, hook_args: tuple[Any, ...]) -> None:
531609
pass
532610
_install_readlink_guard(check_read)
533611
_install_fork_exec_guard(spec.name)
612+
_install_dir_fd_guard()
534613
sys.addaudithook(hook)
535614
try:
536615
result = spec.fn(**args)

tests/test_harness_gate.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,69 @@ def unplant(target: str) -> str:
737737
assert os.path.exists(site_packages_probe), "a site-packages file was deleted from the sandbox"
738738

739739

740+
@_posix
741+
@pytest.mark.parametrize(
742+
"op",
743+
["os.open", "os.mkdir", "os.remove", "os.rename"],
744+
)
745+
def test_gate_dot_pth_cannot_be_planted_via_dir_fd(tmp_path, op, site_packages_probe):
746+
"""The same escape as `test_gate_dot_pth_cannot_be_planted_in_site_packages`,
747+
reached through `dir_fd=` instead of a plain path.
748+
749+
`check_path` resolves the path argument of a call against the process's
750+
cwd (pinned to the workspace). Every one of these calls also accepts a
751+
directory-descriptor keyword that makes the real syscall resolve the same
752+
string against an *open file descriptor* instead — cwd never enters into
753+
it. A tool legitimately opens site-packages for reading (it is in the read
754+
grant, or the import machinery could not work), gets a descriptor for it,
755+
and then `os.open("evil.pth", O_CREAT | O_WRONLY, dir_fd=that_fd)` writes
756+
there while `check_path` validates a path with nothing to do with where
757+
the write actually lands. Verified end to end before this test existed:
758+
the planted `.pth` ran on the very next interpreter start in that
759+
environment — this is not a theoretical gap.
760+
"""
761+
workspace = tmp_path / "ws"
762+
workspace.mkdir()
763+
pth_name = f"grapharc_pwned_dirfd_{uuid.uuid4().hex}.pth"
764+
765+
def plant_via_dir_fd(operation: str, name: str, victim: str) -> str:
766+
import os as _os
767+
768+
dfd = _os.open(_SITE_PACKAGES, _os.O_RDONLY)
769+
try:
770+
if operation == "os.open":
771+
fd = _os.open(name, _os.O_CREAT | _os.O_WRONLY, dir_fd=dfd)
772+
_os.close(fd)
773+
elif operation == "os.mkdir":
774+
_os.mkdir(name, dir_fd=dfd)
775+
elif operation == "os.remove":
776+
_os.remove(_os.path.basename(victim), dir_fd=dfd)
777+
elif operation == "os.rename":
778+
_os.rename(
779+
_os.path.basename(victim), name, src_dir_fd=dfd, dst_dir_fd=dfd
780+
)
781+
return "escaped"
782+
finally:
783+
_os.close(dfd)
784+
785+
harness = _sandbox(workspace, plant_via_dir_fd=plant_via_dir_fd)
786+
try:
787+
with pytest.raises(SandboxViolation, match="directory descriptor"):
788+
harness.call(
789+
"plant_via_dir_fd",
790+
{"operation": op, "name": pth_name, "victim": site_packages_probe},
791+
)
792+
assert not os.path.exists(
793+
os.path.join(_SITE_PACKAGES, pth_name)
794+
), "a file was planted in site-packages via dir_fd"
795+
if op in ("os.remove", "os.rename"):
796+
assert os.path.exists(
797+
site_packages_probe
798+
), "a site-packages file was removed/renamed via dir_fd"
799+
finally:
800+
_scrub("grapharc_pwned_dirfd_") # a regression really does leave one behind
801+
802+
740803
@_posix
741804
@pytest.mark.parametrize(
742805
"op",

0 commit comments

Comments
 (0)