Skip to content

security(rm): add Sandbox.RemoveFile for file-only atomic removal (macOS TOCTOU hardening) #539

Description

@julesmcrt

Background

The rm builtin (added in #537) guards against directory removal without -d using a LstatFile pre-check followed by callCtx.Remove. On Linux this is safe: unlinkat(fd, name, 0) returns EISDIR for any directory, so even if the path is swapped between the two calls, the kernel rejects the Remove. On macOS however, os.Root.Remove has an internal EPERM→AT_REMOVEDIR fallback, meaning it succeeds on empty directories — so a concurrent process that replaces the target file with an empty directory between LstatFile and Remove could cause the directory to be deleted even without -d.

What needs to change

Add a Sandbox.RemoveFile method that calls unlinkat(fd, name, 0) directly (no AT_REMOVEDIR fallback). This makes the "remove a file" operation atomically file-only on all platforms:

  • Linux: returns EISDIR for directories (already true today via os.Root.Remove)
  • macOS: returns EPERM for directories (currently masked by Go's fallback)

Then wire RemoveFile as a second CallContext capability alongside Remove, and use it in the rm builtin when -d is not supplied. Keep Remove (which allows directories) for the -d path.

Scope

  • allowedpaths/sandbox.go: add Sandbox.RemoveFile(path, cwd string) error
  • builtins/builtins.go: add RemoveFile func(ctx context.Context, path string) error to CallContext
  • interp/runner_exec.go: wire RemoveFile in the remediation-mode block
  • analysis/symbols_builtins.go: add "RemoveFile" to callCtxAllFields and builtinPerCommandCallContextFields["rm"]
  • builtins/rm/rm.go: use callCtx.RemoveFile when !allowDir, callCtx.Remove when allowDir
  • Tests: add pentest case that wins the TOCTOU race (swap file→dir between stat and remove) and verifies directory survives

Priority

Low — not exploitable on Linux (production platform). macOS-only risk with a narrow race window and a weak threat model (attacker needs concurrent write access inside AllowedPaths).

Refs: #537 (original rm PR), Codex review comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions