sami/vdm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
Files
963a76b6befffd3904b3c1ac1ad0cb38de7012e8
vdm/daemon/docs/safepath-adversarial.md
T
samiandClaude Sonnet 5 08d7ee9263 daemon: wire the engine into veloxd — the vertical slice runs end to end 
CORE stage 8 merged, so vdm::Engine is linkable. This closes D4a and
narrows D4b: `velox add <url>` now actually downloads.

- sched/engine_port_core.hpp — the real EnginePort: forwards to a live
  vdm::Engine, keeps the DownloadHandle per task for pause/resume/
  cancel/provide_auth/decide/refresh_url, drives set_task_order /
  set_max_active_segments / set_host_segment_cap via
  engine.segment_budget(). CORE confirmed the admission model: DAEMON
  decides when to start(); the engine's own download_task calls
  register_task/set_want internally — DAEMON never touches per-task
  budget calls. EnginePort gains release(TaskId) so the port drops a
  handle when the task goes terminal.
- rpc/event_loop — EventLoop::post(fn): thread-safe, runs fn on the
  loop thread next iteration. The marshaller for engine-thread
  callbacks.
- main.cpp — constructs vdm::Engine + EnginePortCore + Scheduler
  (post_to_loop = loop.post). At startup: reconcile_after_restart()
  (ADR 0013 §5), reload_config(), tick(). A 1 s timerfd on the loop
  re-runs tick() (schedule windows, missed nudges); download.add nudges
  via dispatcher.set_on_mutation.

End-to-end verified against tools/testserver: `velox add
http://127.0.0.1:.../file/512K` -> task queued -> scheduler admits ->
engine downloads 524288 bytes -> complete, file on disk. First
byte-path all the way through the project.

safepath-adversarial.md: re-verified per its own note — CORE landed
O_NOFOLLOW on the target open (core/src/io/sparse_file.cpp), so the
leaf-symlink TOCTOU is now closed; residual is down to one
intermediate-dir gap (documented post-M1 chase).

36 daemon/cli tests green; scheduler + uds_roundtrip TSan-clean.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig
2026-09-10 20:36:11 +04:00

91 lines
8.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# `saveDir` / `filename` → filesystem destination: the adversarial table
`veloxd` is the only process that turns an untrusted string into a place bytes get
written. `capture.offer` means that string can originate from a web page, and
`download.add` over the Unix socket is reachable by any same-UID process. CLAUDE.md §4
("paths are canonicalized and checked against allowed roots before any write") and the M1
DoD ("no path traversal in `saveDir` … → `-32011`") make this a security boundary, not a
formatting nicety.
This table is written **before** `fs/safepath.cpp`, the way EXT did for `shouldCapture`.
Every row is a test in `daemon/tests/safepath_test.cpp`.
Roots for the examples: `allowedRoots = ["/home/u/Downloads", "/data/dl"]`, already
`realpath`-resolved and stored canonical at load time. `$HOME = /home/u`.
| # | Input (`saveDir`, `filename`) | Attack | Required outcome |
|---|---|---|---|
| A1 | `/home/u/Downloads/../.ssh`, `authorized_keys` | `..` climbs out of the root | `-32011`, `data.path` = the input `saveDir`. No dir created. |
| A2 | `/home/u/Downloads/a/b/../../../etc`, `x` | `..` chain escaping after descending | `-32011`. |
| A3 | `/etc`, `cron.d-payload` | absolute path, simply outside every root | `-32011`. |
| A4 | `/home/u/Downloads-evil`, `x` | prefix-match confusion with `/home/u/Downloads` | `-32011` — containment is component-wise, not `starts_with`. |
| A5 | `/home/u/Downloads`, `../.bashrc` | `..` in the **leaf**, not the dir | leaf rejected → `-32011` (or `InvalidParams`); a leaf is one component, never a path. |
| A6 | `/home/u/Downloads`, `sub/dir/file` | `/` in the leaf | leaf rejected — `filename` names a file, not a subpath. |
| A7 | `/home/u/Downloads/link-out` where `link-out` → `/etc` (pre-existing symlink) | symlink component points outside a root | `realpath` resolves it to `/etc`; `-32011`. |
| A8 | `/home/u/Downloads/goodsub`, `iso.img` — but between our check and CORE's `open`, `goodsub` is swapped for a symlink to `/etc` | **TOCTOU** on a directory component | Defense: resolve + create with `openat`/`mkdirat` from an `O_NOFOLLOW|O_DIRECTORY` fd walk, then `realpath` the final dir **again** and re-assert containment. A component that is a symlink at walk time → `-32011`. |
| A9 | `/home/u/Downloads`, `file<NUL>.iso` (`0x00` in the leaf) | NUL truncation — the write path sees `file`, logs/UI see more; CORE's fuzzer hit exactly this via `Content-Disposition` | NUL and every `< 0x20` byte and `0x7F` stripped from the leaf before use (mirrors `core/src/net/content_disposition.cpp` `sanitize_leaf`). If the leaf is empty after stripping → reject. |
| A10 | `/home/u/Downloads`, `"\r\nSet-Cookie: x".iso` | CR/LF injection into logs / downstream | control bytes stripped as A9. |
| A11 | `/home/u/Downloads`, `.` / `..` / `` (empty) | degenerate leaf | rejected. |
| A12 | `/home/u/Downloads`, `con` / `aux` / `nul` | Windows device names | **allowed** on Linux — we are not Windows; do not over-reject. (Noted so a future "harden" pass doesn't add it thinking it was missed.) |
| A13 | `/home/u/Downloads`, `<260 chars>` | overlong leaf, `ENAMETOOLONG` at `open` | leaf capped at 255 **bytes of UTF-8**, never splitting a codepoint (docs/04 §2). |
| A14 | `/home/u/Downloads/<260 chars>/x`, `y` | overlong directory component | `mkdirat` / `realpath` returns `ENAMETOOLONG` → mapped `-32011`, not a crash. |
| A15 | `saveDir` empty / null | no destination given | caller substitutes `saveTo.defaultDir`; `resolve_target` itself rejects an empty dir rather than defaulting silently. |
| A16 | root `/home/u/Downloads` is itself a symlink to `/mnt/big/dl` | a symlinked root | `canonicalize_root` `realpath`s every configured root at load; the stored root is `/mnt/big/dl`, and a `saveDir` resolving there passes. A `saveDir` of the literal `/home/u/Downloads/x` also passes because it `realpath`s to `/mnt/big/dl/x`. |
| A17 | `/home/u/Downloads` exists as a **file**, not a directory | destination is not a directory | `-32011` (`not_a_dir`), no write attempt. |
| A18 | `/home/u/Downloads/新しい/フォルダ`, `映画.mkv` | non-ASCII, legitimate | **succeeds** — UTF-8 is fine; only control bytes and the structural checks apply. |
| A19 | `/home/u/Downloads/./sub/.`, `x` | redundant `.` segments, no escape | normalized away; **succeeds** at `/home/u/Downloads/sub`. |
| A20 | `/home/u/Downloads`, ` trailing-spaces.iso ` / `dots...` | trailing space/dot (Windows-hostile, and confuses "same file" checks) | trimmed: leading/trailing whitespace and trailing dots removed before use. Empty after trim → reject. |
| A21 | relative `saveDir` (`Downloads/x`, `./x`, `x`) | a relative path has no well-defined base and invites cwd games | rejected — `resolve_target` requires an absolute `saveDir`. The GUI/CLI resolve against the default dir before calling. |
## Implementation (`fs/safepath.cpp`, as built)
1. **Sanitize the leaf first**, in isolation: strip `[0x00,0x20) ∪ {0x7F}`, trim
whitespace, strip trailing dots and spaces, reject `.`/`..`/empty/`contains '/'`, cap
255 UTF-8 bytes on a codepoint boundary. (A5, A6, A9–A13, A20)
2. **Require `saveDir` absolute; reject any `..` component lexically.** A legitimate
client never sends `..`; a web-origin path with `..` is an attack, so it does not even
reach `realpath`. (A1, A2, A21)
3. **If the directory already exists:** `realpath(saveDir)` — this follows every symlink,
so a symlinked root or component resolves to where it *really* points — then assert the
resolved path is inside a canonical root, component-wise (`d == root || d starts with
root + "/"`). A symlink that escapes is caught here (A7); one that stays inside passes
(A16). Open the resolved dir `O_PATH|O_DIRECTORY` for the leaf check. (A3, A4, A7, A16,
A17, A19)
4. **If a tail is missing (`mkdir -p` case):** find the deepest existing ancestor,
`realpath` + root-check *that*, then create the missing components through an
`openat/mkdirat` walk with `O_NOFOLLOW|O_DIRECTORY` from the ancestor's fd — the tail
has no symlinks because it had no entries; a race that plants one trips `ELOOP` →
`-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. This narrows the create-after-check race on the leaf; it is fully
closed by CORE opening the download target with `O_NOFOLLOW`. **Verified 2026-09-11:
`core/src/io/sparse_file.cpp` opens `O_WRONLY | O_CREAT | O_CLOEXEC | O_NOFOLLOW`** — a
symlink swapped in as the leaf after our check fails there with `ELOOP` ->
`Error::path_rejected`. (No `O_EXCL`: resume must be able to open an existing
`.veloxpart`.)
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 — one gap, narrowed
**The leaf-symlink TOCTOU is closed** (step 5, verified 2026-09-11: CORE opens the target
`O_NOFOLLOW`). What remains:
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; `O_NOFOLLOW` on the *file* open does not re-check the *directories* above it,
and a full `O_NOFOLLOW` directory chase would reject the legitimate symlinked
directories A16 requires us to allow.
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).
Reference in New Issue View Git Blame Copy Permalink