Cordy/Cairn

Fork 0

Security hardening: tus uploads are not bound to the user who created them #491

New issue

Closed

opened 2026-09-11 21:03:39 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-11 21:03:39 +00:00

Owner

Status: from source (read at bd006ef, not reproduced). The affected code is unchanged on main (e1c855d).

What happens

HEAD, PATCH and DELETE on /api/v1/tus/{id} look an upload up by id only — there is no owner check. finalize writes u.Target through h.Store.Write(r.Context(), …) (internal/tus/tus.go:226–233), i.e. in the storage scope of whoever sends the completing PATCH, not the user who created the upload. With per-user homes, another authenticated user who knows an upload id can:

append bytes into someone else's in-progress upload, or
complete it so the file lands in their own home, or
terminate it.

Mitigating factor

Ids come from crypto/rand (internal/tus/tus.go:309), so this needs a leaked id (proxy or access logs, a shared machine, a client bug). Hence "hardening" rather than an open hole.

Suggested fix

Record the creating user at POST; answer 404 to any other user on HEAD/PATCH/DELETE; always finalize in the creator's scope. Add a two-user test.

Found while building the cairn-desktop tus client (phase 2, Cordy/cairn-desktop#24/#29). Filed by Claude on behalf of @Cordy.

**Status: from source** (read at `bd006ef`, not reproduced). The affected code is unchanged on `main` (`e1c855d`). ## What happens `HEAD`, `PATCH` and `DELETE` on `/api/v1/tus/{id}` look an upload up by id only — there is no owner check. `finalize` writes `u.Target` through `h.Store.Write(r.Context(), …)` (`internal/tus/tus.go:226–233`), i.e. in the storage scope of **whoever sends the completing PATCH**, not the user who created the upload. With per-user homes, another authenticated user who knows an upload id can: - append bytes into someone else's in-progress upload, or - complete it so the file lands in their own home, or - terminate it. ## Mitigating factor Ids come from `crypto/rand` (`internal/tus/tus.go:309`), so this needs a leaked id (proxy or access logs, a shared machine, a client bug). Hence "hardening" rather than an open hole. ## Suggested fix Record the creating user at `POST`; answer `404` to any other user on `HEAD`/`PATCH`/`DELETE`; always finalize in the creator's scope. Add a two-user test. --- Found while building the cairn-desktop tus client (phase 2, Cordy/cairn-desktop#24/#29). Filed by Claude on behalf of @Cordy.

Cordy added this to the v0.7 — Closed beta (dogfood + hardening) milestone

2026-09-11 21:05:56 +00:00

Cordy referenced this issue from a commit

2026-09-12 15:10:56 +00:00

one-shot: #490 os.Root posix + #491 tus owner binding (red-then-green)

forgejo-actions referenced this issue from a commit

2026-09-12 15:20:41 +00:00

#490 #491: posix storage on os.Root (symlink escape closed); tus uploads owner-bound

Cordy referenced this issue

2026-09-12 15:23:08 +00:00

#490 #491: posix storage on os.Root; tus uploads owner-bound #501

Cordy referenced this issue from a commit

2026-09-12 15:25:21 +00:00

Merge pull request '#490 #491: posix storage on os.Root; tus uploads owner-bound' (#501) from fix-490-491 into main

Cordy commented

2026-09-12 15:31:13 +00:00

Author

Owner

Fixed in v0.6.166 (PR #501, live on the dogfood).

As suggested: the creating user is recorded in the upload sidecar at POST; HEAD, PATCH and DELETE from any other user answer the same 404 as an unknown id (no existence oracle), and because only the owner can send the completing PATCH, finalize always runs in the creator's storage scope. Two-user test added (foreign HEAD/PATCH/DELETE all 404, owner still completes), witnessed red first.

Migration note: sidecars written before this release carry no owner and become inert — no authenticated user resolves to the empty owner, so nobody can claim, append to, or terminate them. They cost a few spool bytes until removed by hand; clients simply restart those uploads. (An automatic boot purge was considered and dropped: unauthenticated restart recovery — exercised by the test suite — must keep working, and inert-but-present is equally safe.)

Fixed in **v0.6.166** (PR #501, live on the dogfood). As suggested: the creating user is recorded in the upload sidecar at `POST`; `HEAD`, `PATCH` and `DELETE` from any other user answer the **same 404 as an unknown id** (no existence oracle), and because only the owner can send the completing PATCH, finalize always runs in the creator's storage scope. Two-user test added (foreign HEAD/PATCH/DELETE all 404, owner still completes), witnessed red first. Migration note: sidecars written before this release carry no owner and become **inert** — no authenticated user resolves to the empty owner, so nobody can claim, append to, or terminate them. They cost a few spool bytes until removed by hand; clients simply restart those uploads. (An automatic boot purge was considered and dropped: unauthenticated restart recovery — exercised by the test suite — must keep working, and inert-but-present is equally safe.)

Cordy closed this issue

2026-09-12 15:31:17 +00:00

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Cordy/Cairn#491

No description provided.

Rows
Columns