diff --git a/SECURITY_FIXES_FABLE5.1_IGNORED.md b/SECURITY_FIXES_FABLE5.1_IGNORED.md new file mode 100644 index 0000000..5e68cd2 --- /dev/null +++ b/SECURITY_FIXES_FABLE5.1_IGNORED.md @@ -0,0 +1,190 @@ +# Security findings not fixed (Fable 5.1 pass, 2026-09-02) + +This file records every finding from +`SECURITY_AUDIT_2026-08-28_GEMINI-3.7-HIGH_TEAMWORK.md` and +`security-report/report.md` that was deliberately **not** fixed, or only +partially fixed, and why. The judgement throughout is anchored in what lanspread +is: a desktop launcher for LAN parties where a room of people who know each +other share a fixed, operator-published catalog of games. Peers are +requester-anonymous by design, every byte a peer serves is verified against the +bundled catalog's BLAKE3 authority, and a hostile participant can be unplugged. +Findings that only make sense against an internet-facing, multi-tenant threat +model are noted as such. + +Fixed findings are documented in the individual commits on this branch and are +not repeated here. + +## Gemini audit + +### NET-01 — Mutual TLS for inbound streams (partially fixed) + +- Fixed: forged change hints. A hint is now honoured only when it arrives from + the IP address at which the claimed peer was authenticated, which removes the + reflected state-pull amplification the finding describes. +- Not fixed: requiring client certificates. Requester anonymity is intentional + (documented in the threat model: "responder identity, not membership, is + authenticated"). Anyone on the party LAN running the current build is supposed + to be able to browse and download. mTLS would add certificate handling on + every connection for no gain in that model, because a hostile participant can + still mint a valid self-signed identity. +- The "information harvesting" sub-point (display names, game lists, Call to + Play chat visible to any LAN client) is the product. + +### NET-05 — Collection allocation before semantic validation + +Response frames are capped at 8 MiB and are only read from identity-pinned peers +this node chose to pull from. Deserialising a few tens of megabytes of transient +structures before the bounds check rejects the payload is not a meaningful +resource attack for a desktop app, and the alternative (custom streaming +deserialisers for every collection) would complicate the strict wire codec for +no practical benefit. + +### NET-06 — Unauthenticated chunk and Stream Install egress (informational) + +By design; see NET-01. + +### EXP2-SEC-01 — Post-unpack manifest verification (partially fixed) + +- Fixed: symlink/reparse-point audit before promotion, plus `unrar -ol-`. +- Not fixed: hashing every extracted file against the catalog manifest. The + `.eti` archives are BLAKE3-verified at download time, so their extracted + output is only as trustworthy as the catalog operator, who is trusted by + definition. Stream Install already verifies extracted output because it + receives extracted bytes from a peer; the ordinary path extracts locally from + verified archives. + +### EXP2-SEC-02 — RAR bomb / disk exhaustion + +Same reasoning: a decompression bomb would have to be published in the catalog. +Games are large by nature (the audit's own numbers are hundreds of gigabytes for +a party catalog), and a preflight would need a new dependency for free-space +queries plus a per-game expected size that the ordinary manifest does not carry. +Disk-full during extraction already fails the transaction and rolls staging +back. + +### EXP2-SEC-05 — Pre-verification chunk writes + +Uncommitted downloads are never published: `version.ini` is written only after +every chunk verifies, and the ownership journal plus recovery path handle a +crash mid-download. Buffering 4 MiB per in-flight chunk in memory to avoid +writing bytes that are about to be verified anyway would raise memory use for no +integrity gain. + +### EXP2-SEC-07 — Ambient filesystem calls in `stream_install.rs` + +Architectural parity request. The staging directory is created empty and owned +by the transaction, every path is a validated `CanonicalCatalogPath` resolved by +`validate_relative_path` (which now applies the full portable component rules +and rejects symlink escapes). Refactoring the receiver onto capability handles +is a large change with no concrete exploit behind it. + +### SEC-IPC-01 — Elevated execution of game scripts (partially fixed) + +- Fixed: `sanitize_username` now also strips `& | < > ^` so a batch script's + textual `%~4` expansion cannot chain commands. +- Not fixed: running the scripts elevated, and re-hashing them at launch time. + Elevated setup is the feature: the shared games need registry entries, + redistributables and firewall rules installed. The scripts ship inside catalog + archives that are BLAKE3-verified before installation, so "untrusted P2P batch + scripts" mischaracterises them. Re-verifying the file at launch would only + defend against a local actor who can already write to the user's own game + directory, i.e. who already runs code as that user. +- The proposed `[A-Za-z0-9_-]` username allowlist was rejected because it would + mangle ordinary names (spaces, umlauts). + +### SEC-IPC-04 — Sandboxing `unrar` (partially fixed) + +- Fixed: `-ol-` and the post-extraction link audit. +- Not fixed: Landlock/AppContainer style OS sandboxing of the sidecar. The input + is a catalog-verified archive; a process sandbox for it is not proportionate + to a LAN launcher. Note that the `-sl-` flag the audit recommends does not + exist (`-sl` is a size filter). + +### SEC-IPC-05 — `allow-create-webview-window` capability + +The log windows are opened from the frontend with the app's own URL, and the +companion-window ownership logic is unit-tested TypeScript. The audit found no +script-injection route, and the webview now has a CSP. Moving window creation +into a Rust command to drop one permission is churn for a hypothetical. + +### SEC-FE-01 — ReDoS in the log-window regex filter + +The regex is typed by the local user into their own log viewer. A user can +freeze their own UI thread with `(a+)+$`; nobody else can. Not a security issue +for this app. + +## Codex scan (`security-report/report.md` numbering) + +### [1] Anonymous LAN requesters can monopolize global pools + +Global pools (64 connections, 64 control streams, 48 bulk transfers) are the +intended fairness model for a room of peers; per-origin quotas conflict with the +anonymous requester design, and legitimate use is bursty (ten people pulling the +newest game from one seeder). Discovery now has a per-source-IP budget, which +covers the cheapest flood. A participant who saturates a seeder at a LAN party +is visible and can be dealt with in person. + +### [3] Ordinary install extracts uncatalogued root archives (partially fixed) + +- Fixed: link audit before promotion. +- Not fixed: restricting extraction to the catalog's archive set and verifying + extracted output. The scenario requires the local user to place an extra + `.eti` in their own game directory. See EXP2-SEC-01. + +### [4] Sybil sources can multiply one chunk deadline into a multi-hour loop + +The retry loop tries each distinct peer identity once per chunk by design, +cancellation is always available, and a real party LAN has a handful of peers, +not 64 hostile identities. A global attempt budget would make large legitimate +downloads give up earlier under flaky Wi-Fi. + +### [5] One peer can force continuous rendering of thousands of active calls + +The 4096-event author bound is a wire bound; a peer that publishes thousands of +simultaneous calls degrades the ticker for everyone but this is a prank that the +room can see and stop. Semantic caps on open calls per author would be a product +decision (how many calls may one person host?) rather than a security fix. + +### [7] `justfile` recipe arguments are interpolated as shell code + +The recipes are developer and catalog-operator tooling; the arguments are typed +by the operator on their own machine. Quoting every interpolation risks breaking +the build recipes for a threat that requires the operator to attack themselves. + +### [8] Sybil peers can exhaust aggregate state and unbounded UI publication + +Worst case is 64 authenticated identities each holding a 4 MiB author slice, +i.e. a few hundred MiB retained by a desktop process, and only while a hostile +host keeps dozens of identities alive on the party LAN. Aggregate byte budgets +and bounded latest-value UI channels are a redesign of the state layer, not a +fix, and are not warranted by this deployment model. + +### [10] Mutable game scripts are launched elevated without trust binding + +See SEC-IPC-01. + +### [11] Selected game directories can trigger unbounded monitoring work + +The monitored directory is the one the user selected on their own machine. +Budgeting traversal of the user's own files against the user is out of scope. + +### [12] Public state helpers permit marker writes outside the state directory + +`setup_done_path` and friends join a raw game ID, but every shipping caller +validates the ID as a single path component first (the Tauri commands) or takes +it from the catalog. Making the public helpers fallible would change the crate +API for an embedding caller that does not exist. + +### [13] Catalog preflight can read outside the package root + +Publisher tooling run by the catalog operator over their own package tree; the +disclosed bytes are a bounded, trimmed `version.ini` echoed in the operator's +own error message. + +## Summary + +| Status | Findings | +| --------------- | --------------------------------------------------------------------------------------------------------------------------------- | +| Fixed | NET-02, NET-03, NET-04, EXP2-SEC-03, EXP2-SEC-04, EXP2-SEC-06, SEC-IPC-02, SEC-IPC-03, SEC-DB-01, Codex [2], [6], [9], [14], [15] | +| Partially fixed | NET-01, EXP2-SEC-01, SEC-IPC-01, SEC-IPC-04, Codex [3], [10] | +| Not fixed | NET-05, NET-06, EXP2-SEC-02, EXP2-SEC-05, EXP2-SEC-07, SEC-IPC-05, SEC-FE-01, Codex [1], [4], [5], [7], [8], [11], [12], [13] |