From 7514f5a4c41af5ba75cbe830a48450cf67e939a5 Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 19:49:31 +0400 Subject: [PATCH] daemon: correct an unverified cross-lane claim in safepath-adversarial.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The residual section claimed the leaf/component TOCTOU is "closed in practice by CORE's O_NOFOLLOW open of the final file". Verified: it is not — core/src/io/sparse_file.cpp:77 opens O_WRONLY|O_CREAT|O_CLOEXEC, no O_NOFOLLOW, no O_EXCL. Requested the flags from CORE via PKG/QA. Doc now states the residual is currently OPEN, names the file:line and flags checked and the date, says what actually limits exposure today (0700 parent dirs), and flags this as the boundary where a reader stops checking. Step 5 reworded the same way. Re-verify the flags when the CORE change lands. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/docs/safepath-adversarial.md | 40 +++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/daemon/docs/safepath-adversarial.md b/daemon/docs/safepath-adversarial.md index 3b8559a..5950e07 100644 --- a/daemon/docs/safepath-adversarial.md +++ b/daemon/docs/safepath-adversarial.md @@ -58,20 +58,38 @@ Roots for the examples: `allowedRoots = ["/home/u/Downloads", "/data/dl"]`, alre `-32011`. Then re-derive the final dir's path from its fd (`/proc/self/fd/N`) and re-assert containment. (A8 for the created tail, A14) 5. **Best-effort leaf check:** `fstatat(dir_fd, leaf, AT_SYMLINK_NOFOLLOW)` — refuse if it - is already a symlink. The real close on the create-after-check race is CORE opening the - file `O_NOFOLLOW|O_EXCL` (or `O_NOFOLLOW` + explicit resume); that is CORE's contract, - stated in `daemon/docs/engine-api-review.md`. + is already a symlink. This narrows, but does not close, the create-after-check race on + the leaf: a symlink planted *after* this `fstatat` and *before* CORE opens the file is + still followed. Closing it needs CORE to open with `O_NOFOLLOW` (plus `O_EXCL` on a + fresh download). **Verified 2026-09-10: it does not yet** — + `core/src/io/sparse_file.cpp:77` is `O_WRONLY | O_CREAT | O_CLOEXEC`. The flag change + has been raised with CORE; until it lands this race is open, see the residual below. 6. **Every failure is `-32011`, `data.path` = the *original* `saveDir`** — never the resolved path, which would leak where the roots actually live. The one exception is a `filename` that violates the schema's own `maxLength`, which is `-32602` at the param layer before this code runs. -### Residual, accepted for M1 +### Residual — currently OPEN, tracked -An **existing intermediate directory** swapped for an out-of-root symlink *between* our -`realpath` and CORE's `open` is not caught by this code (step 3 trusts `realpath` for the -pre-existing prefix; a full `O_NOFOLLOW` chase would reject legitimate symlinked -directories mid-path, which A16 requires us to allow). It is closed in practice by CORE's -`O_NOFOLLOW` open of the final file and by the download dir living under a `0700` -`~/.local/share` / `~/Downloads` the attacker would already need write access to. A -per-step "resolve, re-validate against roots" chase is the post-M1 hardening. +Two TOCTOU gaps this code does not close on its own: + +1. **An existing intermediate directory** swapped for an out-of-root symlink between our + `realpath` (step 3) and the write. Step 3 trusts `realpath` for the pre-existing + prefix; a full `O_NOFOLLOW` chase would reject the legitimate symlinked directories + A16 requires us to allow. +2. **The leaf** swapped for a symlink between our `fstatat` (step 5) and CORE's `open`. + +Both are closed by CORE opening the file `O_NOFOLLOW` (and, for a fresh download, +`O_EXCL`). **As verified on 2026-09-10 that is not yet the case** — +`core/src/io/sparse_file.cpp:77` opens `O_WRONLY | O_CREAT | O_CLOEXEC`. The flag change has +been raised with CORE; when it lands, update step 5 and this paragraph and re-verify the +flags at that line. + +What limits the exposure *today*: the download directory lives under `~/.local/share` / +`~/Downloads`, both `0700` — an attacker planting a symlink there already has write access +to the user's account. The unqualified "you are covered" version of this claim does not +hold until the CORE change lands; this is exactly the boundary where a reader stops +checking, so it is spelled out. + +The post-M1 hardening for gap 1 is a per-step "resolve one component, re-validate the +running path against the roots" chase (systemd's `chase_symlinks` shape).