From e86cfc83a3349c5e67e44e610b774b61cbd206c0 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Wed, 2 Sep 2026 22:39:40 +0200 Subject: [PATCH] docs: record security findings deliberately left unfixed Companion to the fix commits on this branch. Lists every finding from the Gemini audit and the Codex scan that was not fixed or only partly fixed, with the reasoning for each, anchored in what lanspread is: a LAN-party launcher with an operator-published, BLAKE3-verified catalog and intentionally anonymous requesters. Claude-Session: https://claude.ai/code/session_017C3Nbgwpdm3YNwZhhFLHwg --- SECURITY_FIXES_FABLE5.1_IGNORED.md | 190 +++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 SECURITY_FIXES_FABLE5.1_IGNORED.md 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] |