Orphan sweep kills on pid+starttime alone, ignoring the namespace inodes it recorded (hardening)

[vyskocilm]

The orphan sweep's kill decision (killOrphanInit, internal/cli/orphansweep.go ~138-160) fires SIGKILL(pid) on exactly two conditions — pid > 1 and procStartTime(pid) == st.Sandbox.InitStarttime — and does NOT verify the target pid lives in the namespaces the state file recorded. The six namespace inodes in runStateSandbox.Namespaces are written by writeRunState and read by attach, but the sweep never cross-checks them, so a state file whose init_pid/init_starttime name any live process is honoured even when that process is in the host's default namespaces and is unrelated to any sandbox.

Found by redteam round against main 022410b. Hardening, not an escape today.

Reproduction (host-side; victim is a sleep the tester started)

VICTIM pid=1092732 starttime=3967283           # plain `sleep 600`, host namespaces
forged: /run/user/1000/snug/target-cb50...41.json   # init_pid=1092732, init_starttime=3967283,
                                                     # namespaces = fabricated inodes 4026534518...
# bin/snug -v <scratch-target> -- /bin/true   (triggers sweepOrphanedSandboxes)
snug: killed orphaned sandbox init pid 1092732 for target /tmp/.../redteam-forged-target-1092724 ...
RESULT: victim pid=1092732 KILLED by the sweep
forged file present after sweep? removed

The victim's real namespaces did not match the fabricated inodes, and the named target directory did not even exist — the kill fired anyway.

What it grants

Nothing new to an attacker under the current threat model. The forged file must be written into /run/user/<uid>/snug, which is uid-private, 0700, and — measured — NOT reachable from inside the sandbox (with default profiles /run does not exist in the sandbox; a write to /run/user/1000/snug/evil.json fails No such file or directory; @tmp-shared binds a hashed per-project subdir of os.TempDir(), not the state dir or a parent). A same-uid host process that can write there can already kill() directly, so the sweep as a kill-proxy adds no privilege. Value is purely defense-in-depth: the sweep is a kill path whose only real safeguard today is "the state dir is uid-private and never bound in."

Severity: sev:low (hardening)

Not an escape. Closes the gap between "the sweep trusts the file" and "the sweep verifies the process is what the file claims." The data to close it is already in the state file and already trusted by attach.

Fix direction (not fixed in this round)

In killOrphanInit, after the starttime check, open /proc/<pid>/ns/pid (optionally ns/mnt) and require its inode to equal the recorded st.Sandbox.Namespaces["pid"] before unix.Kill. Then even a same-uid forged file — or a future profile that accidentally exposed the state dir — cannot turn the sweep into an arbitrary-pid kill.

Regression test (owned by sandbox-tester)

Positive control mirroring the forged-file measurement, via the sweepOrphanedSandboxesIn package-var indirection already exposed for tests: place a target-<sha256(T)>.json in a scratch state dir naming a live sleep the test started, with FABRICATED namespace inodes; run the sweep; assert the sleep survives — i.e. the sweep refuses to kill a pid whose /proc/<pid>/ns/pid does not match the recorded inode. Without the fix, the sleep is killed (current behaviour).

Negatives that held (measured)

• pid reuse: starttime is field 22 of /proc/<pid>/stat in clock ticks since boot (monotonic); a recycled pid's new process necessarily starts after the original init exits, so its starttime is strictly greater and can never equal the recorded value. Parse is the correct last-) form, symmetric writer/reader.
• pid = 0/1/negative: killOrphanInit returns early on pid <= 1.
• pgroup/session widening: kill is unix.Kill(pid, SIGKILL) with the recorded positive pid; no negative-pid/pgid path — blast radius is one pid.
• lock-free / symlink-rename games: lock is an flock on an fd the live snug holds for the whole run, released only on process death; lock+state files live in the uid-private dir the payload cannot reach; dir/file opens go through os.Root + vdir.SecureSubdir ownership/mode checks.
• state dir location: derived from the uid alone (/run/user/<uid>/snug, or /tmp/snug-<uid> only when /run/user/<uid> is absent) — not from $XDG_RUNTIME_DIR/$TMPDIR (the One-sandbox-per-dir fails OPEN when XDG_RUNTIME_DIR/TMPDIR differ between runs: the target lock splits across two inodes and both acquire #122 trap).

Code

internal/cli/orphansweep.go (killOrphanInit ~138-160), internal/cli/runstate.go (procStartTime, runStateSandbox), internal/cli/targetstate.go, internal/cli/targetlock.go, internal/cli/tmpdir.go.