36259b264f
apple / swift (push) Successful in 1m6s
ci / rust (push) Failing after 56s
ci / web (push) Successful in 52s
android / android (push) Successful in 3m24s
ci / docs-site (push) Successful in 1m4s
apple / screenshots (push) Successful in 5m23s
windows-host / package (push) Successful in 7m36s
deb / build-publish (push) Successful in 2m52s
decky / build-publish (push) Successful in 12s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 6s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 4s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 4s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 5s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 4s
ci / bench (push) Successful in 4m43s
rpm / build-publish (bazzite, punktfunk-fedora-rpm) (push) Successful in 8m59s
docker / deploy-docs (push) Successful in 17s
rpm / build-publish (fedora-44, punktfunk-fedora44-rpm) (push) Successful in 8m43s
14/18 fixed (3532e35Linux-verified +6f903f7Windows DACL paths pending CI/box); #5 deferred (needs on-box validation), #9/#13 accepted, S7 acknowledged (no upstream rsa fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
482 lines
83 KiB
Markdown
482 lines
83 KiB
Markdown
# punktfunk host — security audit (2026-06-28, follow-up)
|
||
|
||
> **Status:** AUDIT COMPLETE (2026-06-28). Follow-up to the 2026-06-21 whole-project review
|
||
> ([`security-review.md`](security-review.md)), scoped to the privileged streaming **host**
|
||
> (`crates/punktfunk-host`) — re-verifying the prior 12 findings and hunting the code added since
|
||
> (`library.rs` + store providers, `stats_recorder.rs`, `kwin_fake_input.rs`, session-watch /
|
||
> Desktop↔Game follow, "launch apps on Windows/Linux non-gamescope hosts", "driver/web install into
|
||
> the host exe"). Method: a multi-agent fan-out over **18 attack surfaces** (13 in pass 1 + 5
|
||
> gap-driven in pass 2), every candidate finding **adversarially double-verified** from two
|
||
> independent lenses (reachability/attacker-control + existing-mitigation/correctness), plus a
|
||
> coverage-gap critic. **15 confirmed + 9 partial** issues carried; **8 refuted** recorded for
|
||
> completeness. No memory-unsafety or RCE on attacker wire bytes was found; the residual risk is in
|
||
> dependency hygiene, the opt-in GameStream surface, and Windows local-privilege ACLs.
|
||
|
||
## Remediation status (2026-06-28)
|
||
|
||
Fixes landed on `main` in `3532e35` (Linux/cross-platform, cargo check/clippy/test green here) and
|
||
`6f903f7` (Windows `#[cfg(windows)]` DACL paths — verify in CI / on the RTX box; this Linux dev VM
|
||
can't compile MSVC). Items whose fix would risk a validated pipeline, or that have no upstream
|
||
remedy, are deferred/accepted with a reason.
|
||
|
||
| # | Sev | Status |
|
||
|---|-----|--------|
|
||
| S1 | High | **FIXED** (`3532e35`) — `quinn-proto` → 0.11.15 (RUSTSEC-2026-0185) |
|
||
| #1 | High | **FIXED** (`3532e35`) — unauthenticated nvhttp `GET /pin` removed; PIN only via bearer mgmt API |
|
||
| #2 | High | **FIXED** (`6f903f7`, *Win CI/box pending*) — mgmt token written via `write_secret_file` (SYSTEM/Admins DACL) |
|
||
| #3 | High | **FIXED** (`6f903f7`, *Win CI/box pending*) — config dir DACL-locked + re-owned; `host.env` locked. Residual: a host.env planted before the very first DACL apply is still loaded (an owner-check on load is a noted follow-up) |
|
||
| #4 | High→Med | **FIXED** (`3532e35`) — RTSP/PLAY gated on a paired `/launch` + bound to the launching peer's IP |
|
||
| #5 | Med | **DEFERRED** — the shared-section SDDL is permissive for a restricted-token UMDF driver; scoping it needs on-box validation to avoid breaking the live-validated gamepad/IDD pipeline |
|
||
| #6 | Med | **FIXED** (`3532e35`) — EIS relay moved to `$XDG_RUNTIME_DIR` (0700) + symlink reject |
|
||
| #7 | Med→Low | **FIXED** (`3532e35`) — `vdisplay::ENV_LOCK` serializes setup-path env mutation (data-race UB closed); full per-session `SessionContext` threading for value-confusion is a follow-up |
|
||
| #8 | Low | **FIXED** (`6f903f7`, *Win CI/box pending*) — web-password file created empty → locked → written |
|
||
| #9 | Low | **ACCEPTED** — disarm-on-any-attempt IS the documented single-online-guess (prior-fix #2); the delegated-approval flow is structurally immune. Steer hostile LANs to it |
|
||
| #10 | Low | **FIXED** (`3532e35`) — ENet decrypt-failed warn throttled (exponential) |
|
||
| #11 | Low | **FIXED** (`6f903f7`, *Win CI/box pending*) — logs dir DACL-locked (subsumed by #3) |
|
||
| #12 | Low/Info | **FIXED** (`3532e35`) — parked pairing-waiter cap (+regression test) |
|
||
| #13 | Info | **ACCEPTED** — `PENDING_CAP` + LRU + `requested_at` refresh make an actively-retrying device non-evictable |
|
||
| S2 | Low–Med | **FIXED** (`3532e35`) — a malformed Opus frame drops the frame, keeps the shared mic open |
|
||
| S3 | Low | **FIXED** (`3532e35`) — held buttons/keys are capped `HashSet`s |
|
||
| S4 | Low | **FIXED** (`3532e35`) — Epic launcher-cache reads size-capped |
|
||
| S5 | Low→Info | **FIXED** (`3532e35`) — `fps==0`/absurd rejected at the `open_video` chokepoint |
|
||
| S6 | Low→Info | **FIXED** (`3532e35`) — shared mic mpsc bounded (drop-newest) |
|
||
| S7 | Low→Info | **ACKNOWLEDGED** — `rsa 0.9` Marvin has no fixed upstream release; GameStream is off by default and this is a signing (not decryption-oracle) path. Migrate the GameStream identity to Ed25519/ECDSA when feasible |
|
||
|
||
**Net:** 14 of 18 fixed (5 Linux-verified clusters + 4 Windows DACL paths awaiting CI/box); #5
|
||
deferred pending on-box validation; #9/#13 accepted-with-rationale; S7 acknowledged (no upstream fix).
|
||
|
||
## Consolidated overview & top priorities
|
||
|
||
The host's **core trust architecture remains sound**: native SPAKE2 pairing (single-use
|
||
disarm-before-verify, CSPRNG PIN, sanitized device names, atomic+rollback persist), post-pair
|
||
cert-pinning that verifies the real `CertificateVerify` signature, the management API authn/authz
|
||
split (read-only-cert allowlist vs. bearer-gated mutations), uniformly bounds-checked client→host
|
||
wire decoders (no reachable parse panic/OOB), memory-safe client-geometry→encoder/FFI paths, a clean
|
||
driver-IPC ABI, and a fail-closed app-layer pairing gate. The new library/launch surface is notably
|
||
well-defended against the network adversary (client ids resolve against the host's own catalog,
|
||
argv-only, no shell, **no SSRF**). Most prior fixes are present and not regressed.
|
||
|
||
The real risk clusters in **three** places: (1) a **vulnerable QUIC dependency on the always-on
|
||
default listener**, (2) the **opt-in GameStream/Moonlight compatibility surface** (two pre-auth
|
||
boundary bypasses), and (3) **Windows `%ProgramData%` ACLs** (the prior secret-file fix did not cover
|
||
the directory or two newer writers).
|
||
|
||
**Fix promptly (priority order):**
|
||
|
||
| P | Finding | Sev | Auth | Surface |
|
||
|---|---------|-----|------|---------|
|
||
| 1 | **S1** `quinn-proto 0.11.14` (RUSTSEC-2026-0185) → pre-auth remote memory-exhaustion DoS on the **default** `serve` QUIC listener | High | pre-auth | dep / native QUIC |
|
||
| 2 | **#1** Unauthenticated GameStream `GET /pin` → full pre-auth self-pairing (consent bypass) → capture + input injection | High | pre-auth | GameStream (opt-in) |
|
||
| 3 | **#2** Windows mgmt bearer token written without DACL — any local user reads the admin credential | High | local | secrets |
|
||
| 4 | **#3** `%ProgramData%\punktfunk` dir + `host.env` not DACL-locked → local user → SYSTEM env/arg injection (LPE) | High | local | Windows service |
|
||
| 5 | **#4** Pre-auth RTSP/UDP media plane has no pairing gate → desktop disclosure (portal) + stream-slot DoS | High→Med | pre-auth | GameStream (opt-in) |
|
||
|
||
**Medium:** **#5** Windows gamepad/IDD shared sections `Everyone:GENERIC_ALL` (local input-inject /
|
||
screen read) · **#6** gamescope EIS socket via predictable `/tmp` relay (local keylog / input DoS) ·
|
||
**#7** process-global env retargeting unsound under default concurrent sessions (`set_var`/`getenv`
|
||
data-race UB → host-wide DoS; the live form of deferred prior-fix #7) · **S2** malformed client Opus
|
||
frame tears down the shared host-lifetime virtual mic (cross-session DoS).
|
||
|
||
**Low / info:** **#8** `web-password` write-then-`icacls` TOCTOU · **#9** pairing-window-burn DoS ·
|
||
**#10** ENet control-flood warn-log spam · **#11** SYSTEM `host.log` link-redirection (sub-case of
|
||
#3) · **#12** legacy pairing no rate-limit · **#13** pending-approval queue flood · **S3** unbounded
|
||
held-button/key `Vec` growth · **S4** unbounded read of Epic launcher caches · **S5** refresh/fps
|
||
lower-bound unvalidated on the Hello path (self-inflicted single-session panic) · **S6** unbounded
|
||
mpsc into the shared mic service · **S7** `rsa 0.9` Marvin advisory on the opt-in GameStream signing
|
||
path (not practically reachable).
|
||
|
||
**Highest-leverage remediations** (each closes a cluster): (a) `cargo update -p quinn-proto
|
||
--precise 0.11.15` + wire `cargo audit` into CI as a failing gate; (b) delete the unauthenticated
|
||
nvhttp `/pin` and bind RTSP/PLAY to a paired `/launch` session; (c) DACL-lock the Windows config
|
||
directory and route **all** config/secret writes through `write_secret_file`; (d) thread per-session
|
||
launch/compositor/input env through `SessionContext` instead of process-global `std::env`.
|
||
|
||
---
|
||
|
||
The two passes' full verified detail follows verbatim (pass 1 = the 13-surface report; pass 2 = the
|
||
supplement completing the native-protocol/unsafe-FFI surfaces + coverage-critic gaps), then the
|
||
coverage-gap appendix.
|
||
|
||
---
|
||
|
||
# Pass 1 — 13-surface report
|
||
|
||
# punktfunk host — security audit (2026-06-28, follow-up)
|
||
|
||
**Status:** Follow-up audit of the privileged streaming host (`crates/punktfunk-host`), focused on code added since the 2026-06-21 review (`library.rs` + store providers, `stats_recorder.rs`, `kwin_fake_input.rs`, session-watch/Desktop-Game follow, the "launch apps on Windows/Linux non-gamescope hosts" path, and the "move driver/web install into the host exe" path), plus a regression re-verification of the prior twelve findings. Thirteen surface areas reviewed; every candidate finding was adversarially double-verified. **9 confirmed + 4 partial** issues are carried; **6 refuted** items are recorded for completeness.
|
||
|
||
## Executive summary
|
||
|
||
The host's core trust architecture remains sound: the native SPAKE2 pairing ceremony, the post-pair mTLS cert-pinning model, the management API authn/authz split (read-only cert allowlist vs. bearer-gated mutations), and the RTSP/input/gamepad wire parsers are all carefully hardened and, where re-verified, the prior fixes are present and not regressed. The new game-library/launch surface is notably well-defended against the network adversary — client-supplied launch ids are resolved against the host's own scanned catalog, numeric/charset-validated, and spawned argv-based (no shell) on every non-operator path.
|
||
|
||
The real risks cluster in two places. **First, the opt-in GameStream/Moonlight compatibility surface (`serve --gamestream`) deviates from its own trust boundary in two pre-auth ways:** the legacy nvhttp `GET /pin` endpoint is completely unauthenticated, letting an unpaired LAN peer drive the *entire* pairing ceremony with no operator consent and obtain a persistent paired identity with full capture + input injection (Finding 1, the single highest-leverage issue); and the RTSP/UDP media plane performs no pairing/launch check at all, so an unpaired peer can start capture/encode and receive the desktop stream (Finding 4). Both are gated only by the opt-in `--gamestream` flag and the documented "trusted-LAN-only" posture — but within that supported mode they are genuine pre-auth bypasses of the pairing boundary that `/launch` otherwise enforces.
|
||
|
||
**Second, the Windows LocalSystem service has three local-privilege gaps rooted in one cause — the prior fix #1 hardened secret *files* but not the `%ProgramData%\punktfunk` *directory* or two newer files written into it.** The management bearer token is written with no Windows DACL (Finding 2), and `host.env` — which feeds the SYSTEM service's environment and command-line arguments — is neither DACL-locked nor is its directory (Finding 3). These give a local unprivileged user a path to the admin management plane and, via directory pre-creation / env injection, toward SYSTEM. On Linux/gamescope, a world-readable `/tmp` EIS-socket relay lets a second local user keylog or deny the remote session's input (Finding 6). The remaining items are lower-severity local IPC ACL over-breadth (gamepad shared memory), a concurrency-introduced `std::env::set_var` data race that is now reachable because concurrent native sessions became the default (Finding 7, the live form of deferred prior-fix #7), and pre-auth DoS edges.
|
||
|
||
Overall posture is good and improving; the GameStream pairing/media pre-auth bypasses and the Windows config-directory ACL gap are the items that warrant prompt remediation.
|
||
|
||
## Findings
|
||
|
||
| # | Severity | Surface | Title | Status |
|
||
|---|----------|---------|-------|--------|
|
||
| 1 | High | GameStream pairing | Unauthenticated nvhttp `GET /pin` → full pre-auth GameStream self-pairing (consent bypass) | Confirmed |
|
||
| 2 | High | Secrets / mgmt | Windows mgmt bearer token written without DACL — local-user disclosure of host admin credential | Confirmed |
|
||
| 3 | High | Windows service / config | `%ProgramData%\punktfunk` directory + `host.env` not DACL-locked → local user → SYSTEM env/arg injection | Confirmed (apps.json sub-vector: Partial) |
|
||
| 4 | High→Med | GameStream RTSP/media | Pre-auth RTSP ANNOUNCE+PLAY starts capture/encode with no pairing gate (desktop disclosure + stream-slot DoS) | Partial |
|
||
| 5 | Medium | Input injection | Windows host↔UMDF gamepad shared sections are `Everyone:GENERIC_ALL` — local cross-session input injection/tamper | Confirmed |
|
||
| 6 | Medium | Session lifecycle (gamescope) | EIS socket path relayed via predictable world-accessible `/tmp` file — local keylog / input DoS | Confirmed |
|
||
| 7 | Medium→Low | Session lifecycle | Process-global env retargeting unsound under now-default concurrent native sessions (data race + cross-session confusion) | Confirmed |
|
||
| 8 | Low | Secrets | `web-password` written world-readable then `icacls`'d — brief TOCTOU disclosure | Confirmed |
|
||
| 9 | Low | Native pairing | Unpaired LAN peer can burn the operator's single-use pairing window (pairing-ceremony DoS) | Confirmed |
|
||
| 10 | Low | GameStream control | ENet control flood → unbounded per-packet warn-log spam (+ transient CPU) | Confirmed |
|
||
| 11 | Low→Info | Windows service | SYSTEM `host.log` predictable name in Users-writable dir (link-redirection of SYSTEM appends) | Partial |
|
||
| 12 | Low→Info | GameStream pairing | Legacy pairing has no rate-limit; parks unbounded 300 s waiters | Partial |
|
||
| 13 | Info | Native pairing | Pending-approval queue floodable by LAN cert flood (eviction of a genuine knock) | Confirmed |
|
||
|
||
---
|
||
|
||
## Finding details (confirmed & partial)
|
||
|
||
### 1. [High] Unauthenticated nvhttp `GET /pin` enables full pre-auth GameStream self-pairing — *Confirmed*
|
||
|
||
- **Surface:** GameStream pairing ceremony / nvhttp.
|
||
- **Refs:** `gamestream/nvhttp.rs:61`, `nvhttp.rs:85-96` (`h_pin`, plain-HTTP router), `gamestream/pairing.rs:40-43` (`PinGate::submit`), `pairing.rs:102-150` (`getservercert`), `pairing.rs:226-234` (phase 4 / `save_paired`), `crypto.rs:35-40` (`pin_key`).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1). Requires `serve --gamestream`.
|
||
- **Mechanism:** The GameStream PIN is the sole proof of operator consent (`aes_key = SHA-256(salt ‖ pin)`), and the host has no independent knowledge of the correct PIN — it derives the key from whatever is delivered to `PinGate::submit`. The operator-channel (`mgmt` `POST /api/v1/pair/pin`) is bearer-gated for exactly this reason, **but the host also exposes `GET /pin?pin=NNNN` on the unauthenticated nvhttp router with no auth and no `awaiting_pin` guard**, on `0.0.0.0:47989` (plain HTTP) and `:47984`. Because the attacker controls both the `getservercert` request (its own salt + cert) *and* can submit the PIN itself, it supplies both sides of the ceremony. There is no operator "arm pairing" gate for the legacy GameStream path (unlike native SPAKE2).
|
||
- **Attack scenario:** (1) Attacker sends `GET /pair?phrase=getservercert&uniqueid=X&salt=<32hex>&clientcert=<own-cert-hex>` → parks on `pin.take(300s)`. (2) Attacker sends unauthenticated `GET /pin?pin=4242` → the parked `take()` returns it; host computes `aes_key = SHA-256(attacker_salt ‖ "4242")`, which the attacker also knows. (3) Attacker completes `clientchallenge`/`serverchallengeresp`/`clientpairingsecret` (all derivable — it knows the key and owns its cert); phase 4 pins the attacker cert via `save_paired`. (4) Attacker reconnects over HTTPS:47984 with its now-pinned cert; `peer_is_paired()` is true → `/launch` + `/applist` succeed → desktop capture and keyboard/mouse/gamepad injection on the privileged host. **No operator action at any step.**
|
||
- **Existing mitigations:** GameStream is opt-in and documented "trusted-LAN only"; default `serve` does not start nvhttp. The post-pair launch surface is correctly gated by `peer_is_paired` — it just gets satisfied because the attacker self-pairs. None of these is a control on `/pin`.
|
||
- **Verifier adjudication:** Both verifiers **confirmed reachable + attacker-controlled**, downgrading the original *critical* to **high** only because the surface is the opt-in, documented-weaker `--gamestream` mode (smaller affected population than the always-on native listener). This is **not** subsumed by accepted-risk #9 (which covers `/pair` being plain HTTP / a MITM brute-force, not unauthenticated PIN self-delivery).
|
||
- **Recommendation:** Remove the unauthenticated nvhttp `GET /pin` endpoint entirely; PIN delivery must come only from the bearer-gated mgmt API. If a nvhttp delivery path must remain, require an explicit operator "arm GameStream pairing" step (mirror native `native_pairing` arm-on-demand) and bind the submitted PIN to that armed window. Ideally have GameStream pairing display a *host-generated* PIN the operator confirms, rather than accepting an arbitrary client-side PIN.
|
||
|
||
---
|
||
|
||
### 2. [High] Windows mgmt bearer token written without DACL lockdown — *Confirmed*
|
||
|
||
- **Surface:** Secret-file permissions / management authz. (Reported independently by two surface auditors; same defect.)
|
||
- **Refs:** `mgmt_token.rs:59-71` (`write_token`), `mgmt_token.rs:40-44` (dir via `fs::create_dir_all`), `gamestream/mod.rs:251-261` (`config_dir` = `%ProgramData%\punktfunk`), `gamestream/mod.rs:282-285` (`create_private_dir` is a no-op for ACLs on Windows), `gamestream/mod.rs:293-347` (`write_secret_file`/`restrict_to_system_admins`).
|
||
- **Threat actor:** Local unprivileged user (#4). Windows host only (Unix is correctly `O_CREAT 0600`).
|
||
- **Mechanism:** The mgmt bearer token grants full admin authority over the management API. It is persisted by `write_token`, the **only** host secret writer that does not route through `write_secret_file → restrict_to_system_admins`; it applies a Unix `0600` mode but has **no `#[cfg(windows)]` arm**. On the LocalSystem service, `config_dir()` is `%ProgramData%\punktfunk`, whose inherited default DACL grants `BUILTIN\Users` read; `create_private_dir` applies no DACL on Windows and explicitly relies on each secret file being individually locked by `write_secret_file`. The token file is therefore left Users-readable. (The host key, cert, and both trust stores *are* locked — the token is the regressed outlier; the `write_secret_file` doc comment ironically claims it "Mirrors the mgmt-token hardening.")
|
||
- **Attack scenario:** A local unprivileged user reads `C:\ProgramData\punktfunk\mgmt-token`, then presents `Authorization: Bearer <token>` to the loopback mgmt HTTPS API (default `127.0.0.1:47990`; self-signed cert trivially ignored). They now hold full admin authority: arm native pairing and read the PIN, approve their own device into the paired trust store, unpair/add clients, control sessions, and `POST /library/custom` with a `command` LaunchSpec that the host subsequently executes — a plausible path to code execution beyond the user's own privileges.
|
||
- **Existing mitigations:** Default bind is loopback; API still requires HTTPS+bearer — but that bearer is exactly what leaks. The sibling `web-password` *is* `icacls`-hardened (`install.rs:280-289`), confirming this is a missed file, not a design choice.
|
||
- **Verifier adjudication:** Both verifiers (across two surfaces) **confirmed at high**; this is the same class/severity as prior HIGH #1 (host key readable by any local user) and a genuine regression of that principle. `attacker_controlled=false` correctly reflects that this is a credential disclosure, not value injection.
|
||
- **Recommendation:** Route the mgmt-token write through `gamestream::write_secret_file` (or call `restrict_to_system_admins` on the path after writing) and create the dir with `create_private_dir`'s Windows DACL. Re-tighten any pre-existing token file on startup.
|
||
|
||
---
|
||
|
||
### 3. [High] Windows config directory and `host.env` are not DACL-locked → local user → SYSTEM env/arg injection — *Confirmed* (apps.json sub-vector *Partial*)
|
||
|
||
- **Surface:** Windows LocalSystem service / config & discovery. (Merges the `host.env` finding and the config-directory finding — same root cause.)
|
||
- **Refs:** `windows/service.rs:681-713` (`ensure_default_host_env` plain `std::fs::write`, skips if file `exists()`), `service.rs:159-180` (`load_host_env` `set_var`s *every* KEY=VALUE, not just `PUNKTFUNK_*`), `service.rs:301-302` (`format!("\"{}\" {host_cmd}", exe)` from `PUNKTFUNK_HOST_CMD`), `gamestream/mod.rs:264-286` (config dir never DACL-locked; `create_private_dir` no-op on Windows), `gamestream/apps.rs:40-95` + `stream.rs:140-145` (apps.json `cmd`).
|
||
- **Threat actor:** Local unprivileged user (#4). Windows host only.
|
||
- **Mechanism:** Secret *files* are individually `icacls`-locked, but the `%ProgramData%\punktfunk` *directory* is never DACL-restricted and `host.env` is written with a bare `std::fs::write`. Under the default `%ProgramData%` ACL, `BUILTIN\Users` inherit a container "create folders" right (and become `CREATOR OWNER` of subfolders they create). A non-admin who pre-creates the `punktfunk` subfolder before the elevated installer/service populates it owns it with full control and can plant `host.env`/`apps.json`; `ensure_default_host_env` then skips writing because the file already `exists()`. On service start, `load_host_env` injects every line of `host.env` into the SYSTEM process environment, and `supervise()` builds the SYSTEM child command line verbatim from `PUNKTFUNK_HOST_CMD`.
|
||
- **Attack scenario / impact:** The surviving primitives (after verifier scrutiny) are: (a) **arbitrary SYSTEM-process environment injection** — e.g. set `PATH`/DLL-search vars in `host.env` to an attacker-writable directory and plant a hijackable DLL the SYSTEM host loads by name; (b) **attacker-controlled SYSTEM argv** to the fixed signed `punktfunk-host.exe`; (c) config-dir/trust-store tampering. Each independently sustains a **local privilege escalation toward NT AUTHORITY\SYSTEM**. The planted-`apps.json` `cmd` vector is weaker than originally stated: `launch_gamestream_command` → `interactive::spawn_in_active_session` runs the cmd under the **interactive console user** token (`WTSQueryUserToken`+`CreateProcessAsUserW`), not SYSTEM — so apps.json planting yields code execution *as the interactive user*, not SYSTEM.
|
||
- **Verifier corrections:** The literal `PUNKTFUNK_HOST_CMD=... & malware.exe` shell-injection payload does **not** work — `spawn_host` uses `CreateProcessAsUserW` with no shell, so `&` is an inert argv token. Exploitation is gated on **directory pre-creation** (the punktfunk subfolder must be absent at attack time — fresh install before first launch, or a removed dir); on a normally installed box the elevated installer/SYSTEM service owns the dir and the default ACL grants Users *create-subdirectory* but not *create-file*, blocking overwrite of an existing admin-owned `host.env`. One verifier adjusted to **medium** on these grounds; the other held **high**. Carried at **high** because the env/arg-injection LPE primitives are real and the directory is genuinely never re-secured.
|
||
- **Existing mitigations:** Secret files are DACL-locked individually; the elevated installer creates the dir in normal flows. GameStream/apps.json launch is opt-in and additionally needs a launch to occur.
|
||
- **Recommendation:** Apply a restrictive DACL to the config directory at creation on Windows (`SYSTEM`/`Administrators` full + `CREATOR OWNER`, strip inheritance) inside `create_private_dir`; write `host.env` through `write_secret_file`; and refuse to load `host.env`/honor `PUNKTFUNK_HOST_CMD` (and trust `apps.json` `cmd`) unless the file/dir is owned by SYSTEM/Administrators.
|
||
|
||
---
|
||
|
||
### 4. [High→Medium] Pre-auth RTSP/UDP media plane has no pairing gate — *Partial*
|
||
|
||
- **Surface:** GameStream RTSP / video stream.
|
||
- **Refs:** `gamestream/rtsp.rs:91` (`handle_conn`, no auth), `rtsp.rs:204-216` (ANNOUNCE writes `state.stream` unauthenticated), `rtsp.rs:218-239` (PLAY starts video on `Some(cfg) && !streaming.swap(true)`, never checks `state.paired`/`state.launch`), `gamestream/stream.rs:90-108` (UDP 47998 binds and `connect()`s the first pinger), `gamestream/mod.rs:214` (`rtsp::spawn` only under `--gamestream`).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1). Requires `serve --gamestream`.
|
||
- **Mechanism:** nvhttp gates `/launch`/`/applist`/`/resume`/`/cancel` on `peer_is_paired()`, but the RTSP listener (TCP 48010) and the UDP media planes are unauthenticated. `ANNOUNCE` stores a client-chosen `StreamConfig` (width/height/fps/codec/packetSize) with no auth; `PLAY` starts the video stream consulting neither `state.paired` nor `state.launch` (only the optional audio sub-stream requires the launch `gcm_key`). Video is sent in plaintext, so no key is needed. There is no per-launch session token and no binding between the paired nvhttp client and the RTSP/UDP peer (unlike Sunshine, which validates the launch session).
|
||
- **Attack scenario:** Unpaired attacker sends a minimal ANNOUNCE SDP → host stores a config; sends PLAY → host spawns the video pipeline, detects the compositor, creates a virtual output / opens the encoder; sends any UDP datagram to 47998 → host `connect()`s there and streams. Net effects: (a) **pre-auth desktop disclosure** — full real-monitor leak on the `PUNKTFUNK_VIDEO_SOURCE=portal` path; on the recommended `virtual` source the attacker captures a *fresh blank* virtual output (no app, since `/launch` is pairing-gated), and the default source is a synthetic test pattern; (b) **unconditional pre-auth resource consumption** (forces virtual-output creation + GPU encode); (c) **stream-slot DoS** — `streaming.swap` allows only one stream, so an attacker can grab and hold the slot against legitimate clients (an in-progress legit session cannot be concurrently hijacked).
|
||
- **Existing mitigations:** Opt-in `--gamestream`; documented trusted-LAN-only; `streaming.swap` single-stream lock; packetSize bounded `[64,2048]`; `encode::validate_dimensions` bounds ANNOUNCE width/height. **None is an authentication check on the media plane.**
|
||
- **Verifier adjudication:** Both verifiers confirmed the bypass is real and unconditional; severity split **high vs. medium** turning on the capture source (portal = real-desktop leak → high; virtual/default = blank/test-pattern, leaving DoS + boundary bypass → medium). Carried at **high→medium**: the pairing authz boundary is unconditionally bypassed and the portal path leaks the real desktop, but the most-common `virtual` configuration limits disclosure.
|
||
- **Recommendation:** Require a valid recent `/launch` session (set by a paired HTTPS client) before ANNOUNCE/PLAY will start a stream, and bind the RTSP/UDP peer to the launching client's address / a per-launch session secret (as Sunshine does). At minimum, refuse PLAY when `state.launch` is `None` and no paired client has an active session.
|
||
|
||
---
|
||
|
||
### 5. [Medium] Windows host↔UMDF gamepad shared sections are world-writable (`Everyone:GENERIC_ALL`) — *Confirmed*
|
||
|
||
- **Surface:** Input injection (Windows virtual-pad IPC).
|
||
- **Refs:** `inject/windows/gamepad_raii.rs:43` (SDDL literal `D:(A;;GA;;;WD)`), `gamepad_raii.rs:37-81` (`Shm::create`), `inject/windows/dualsense_windows.rs:239`, `dualshock4_windows.rs:40`, `gamepad_windows.rs:158`; same SDDL at `capture/windows/idd_push.rs:245` (`Global\pfvd-*` frame textures).
|
||
- **Threat actor:** Local unprivileged user (#4). Windows host only.
|
||
- **Mechanism:** Every virtual-pad backend creates its host↔driver section in the kernel `Global\` namespace with a SECURITY_ATTRIBUTES built from `D:(A;;GA;;;WD)` — `WD` = Everyone (S-1-1-0), `GA` = GENERIC_ALL — and **no mandatory integrity label** (so the SYSTEM-created object defaults to medium IL / `NO_WRITE_UP` only). The host writes the live HID input report into `OFF_INPUT`; the privileged UMDF driver streams those exact bytes to games as virtual-controller input. The DACL grants full access to Everyone, so any interactive medium-IL local user can `OpenFileMapping("Global\pfds-shm-0", FILE_MAP_WRITE)` while a session has a pad active.
|
||
- **Attack scenario:** A separate unprivileged local account (different session / fast-user-switch / RDP) opens the named section and overwrites `OFF_INPUT` with attacker-chosen button/stick/trigger values → the driver injects them into the streaming user's game. It can also corrupt the magic/`device_type` (DoS / device confusion) and observe the streaming user's input. The identical SDDL on `idd_push.rs` additionally lets any local user **read captured screen frames**.
|
||
- **Existing mitigations:** `Global\` creation needs `SeCreateGlobalPrivilege`, preventing pre-creation/squatting — but **opening** an existing object only needs DACL access. The section exists only while a pad is active; keyboard/mouse use `SendInput` (not this channel), so injection is gamepad-only.
|
||
- **Verifier adjudication:** Both verifiers **confirmed at medium** — genuine cross-session/cross-privilege input injection + IPC tamper + (via the shared SDDL) screen-content disclosure; bounded below high by being local-only, needing a concurrent local account and a live pad.
|
||
- **Recommendation:** Scope the section DACL to exactly the principal the WUDFHost runs as (grant SYSTEM and the specific WUDF/driver service SID) instead of `Everyone`, and add a mandatory label / deny lower-IL writers (e.g. replace `WD` with the WUDFHost service account SID + `S:(ML;;NW;;;ME)`). Apply the identical fix to the `Global\pfvd-*` frame-texture sections in `capture/windows/idd_push.rs`.
|
||
|
||
---
|
||
|
||
### 6. [Medium] Gamescope EIS socket path relayed through a predictable, world-accessible `/tmp` file — *Confirmed*
|
||
|
||
- **Surface:** Session lifecycle / libei input injection (gamescope backend).
|
||
- **Refs:** `vdisplay/linux/gamescope.rs:778` (`EI_SOCKET_FILE = /tmp/punktfunk-gamescope-ei`), `gamescope.rs:797` (`remove_file`, error ignored), `gamescope.rs:807` (`printf %s "$LIBEI_SOCKET" > /tmp/...`), `gamescope.rs:677` (`fs::write`), `inject/linux/libei.rs:298-345` (`connect_socket_file`: `read_to_string` + `UnixStream::connect`, no ownership/symlink/stat check), `libei.rs:193` (wiring).
|
||
- **Threat actor:** Local unprivileged user (#4). Gamescope hosts only (Steam Deck / Bazzite gaming mode, or `PUNKTFUNK_COMPOSITOR=gamescope`). KWin/Mutter/Sway use D-Bus `ConnectToEIS` and are unaffected.
|
||
- **Mechanism:** The nested session writes gamescope's `LIBEI_SOCKET` path to the fixed world-readable `/tmp/punktfunk-gamescope-ei`. The libei injector reads that file and `UnixStream::connect`s to whatever absolute path it contains — **with no verification that the file or target socket is owned by the host uid** — then streams the remote client's keyboard/mouse events to it as a libei client. EIS has no peer authentication, so a fake server captures the input stream. On sticky `/tmp` (1777), if a different uid pre-creates the relay file (owner=attacker, mode 0644), the host's `remove_file` and `> file`/`fs::write` truncate both fail (EPERM/EACCES, errors ignored), so the attacker's content survives and the host connects to the attacker's socket. `stop_session` removes the host-owned file on each teardown, giving a recurring re-plant window.
|
||
- **Attack scenario:** Local attacker runs `echo /home/attacker/evil.sock > /tmp/punktfunk-gamescope-ei` (0644) and listens on `evil.sock` as an EIS server. When a remote client streams, the injector connects there instead of gamescope's real EIS; every keystroke/pointer event the remote user sends (game/Steam input, typed credentials) is delivered to the attacker, and gamescope receives no input (input DoS).
|
||
- **Existing mitigations:** The real EIS socket lives under `XDG_RUNTIME_DIR` (0700) — but only its *path* is leaked/overridable via the `/tmp` relay. `protected_symlinks` does not help (regular file, not symlink). The injector retries only `ConnectionRefused`/`NotFound`; a live attacker socket returns `Ok` and is trusted.
|
||
- **Verifier adjudication:** Both verifiers **confirmed at medium**. Impact is high (full remote keystroke capture incl. credentials, plus input DoS) but local-only, gamescope-backend-only, and most gamescope deployments are single-user, capping practical likelihood.
|
||
- **Recommendation:** Relay the EIS path through a host-private location (a file in `XDG_RUNTIME_DIR`, 0700, created `O_EXCL`) instead of `/tmp`, and/or `stat` the relay file and reject it unless owned by the host uid, mode ≤0644, not a symlink, before reading. Apply the same hardening to the predictable world-readable `/tmp/punktfunk-gamescope.log`.
|
||
|
||
---
|
||
|
||
### 7. [Medium→Low] Process-global env retargeting is unsound under now-default concurrent native sessions — *Confirmed*
|
||
|
||
- **Surface:** Session lifecycle / library-launch. (Merges the "native concurrent launch-env race" and the "apply_session_env/apply_input_env" findings — one root cause; the live, generalized form of deferred prior-fix #7.)
|
||
- **Refs:** `punktfunk1.rs:150` (`DEFAULT_MAX_CONCURRENT=4`), `punktfunk1.rs:254` (Semaphore), `punktfunk1.rs:612` (`std::env::set_var("PUNKTFUNK_GAMESCOPE_APP", &cmd)`), `punktfunk1.rs:1871`/`1885` (`apply_session_env`/`apply_input_env` calls), `vdisplay.rs:367-397`/`457-485` (env setters), `vdisplay/linux/gamescope.rs:791-794` (reads the global env), `punktfunk1.rs:600`/`vdisplay.rs:363-365` (stale "ONE-session-at-a-time" comments).
|
||
- **Threat actor:** Malicious network client, **post-auth** (#2, paired/trusted-tier).
|
||
- **Mechanism:** The native host now serves up to 4 concurrent sessions by default, yet the per-session handshake mutates *process-global* environment via `std::env::set_var` (resolved launch id into `PUNKTFUNK_GAMESCOPE_APP`; plus `WAYLAND_DISPLAY`/`XDG_RUNTIME_DIR`/`DBUS_SESSION_BUS_ADDRESS`/`PUNKTFUNK_INPUT_BACKEND`/etc. via `apply_session_env`/`apply_input_env`). These run inside `spawn_blocking` for each concurrent handshake and are then read by backends/injectors at open time. The in-code invariant ("the host serves one session at a time, so a process-global write is sound") is now false. Two effects: (1) **concurrent `set_var` while another thread `getenv`s is documented UB in Rust** (glibc `environ` realloc) → potential host-wide crash taking down all live sessions; (2) session B's handshake overwrites the env session A's gamescope-spawn/injector is about to read → A launches B's (operator-approved) title or routes input to B's backend.
|
||
- **Attack scenario:** Two paired clients connect concurrently (or one reconnects in a tight loop while another session is active). The racing `set_var`/`getenv` can abort the host (DoS affecting all sessions); concurrently A's session can be mispointed.
|
||
- **Verifier adjudication:** Both **confirmed** the technical defect; severity split **medium vs. low**. The cross-session *launch/input misrouting* grants no new authority (both peers are already authorized to view/inject on the shared desktop; the `uid` filter prevents cross-user selection), so under "only NEW authority counts" it is a correctness bug. The surviving security impact is the **`set_var`/`getenv` data-race UB → non-deterministic host-wide DoS**, triggerable by an already-paired device. Carried at **medium→low** accordingly.
|
||
- **Existing mitigations:** Pairing gate runs before `resolve_compositor` (post-auth). `detect_active_session` filters `/proc` by the host's own uid (no cross-user selection). Most env writes are gated to auto-detect mode (skipped when `PUNKTFUNK_COMPOSITOR` is set). No lock serializes the env writes, and there is no per-session config object for these knobs (unlike the Windows/GameStream `SessionContext.launch`).
|
||
- **Recommendation:** Stop using process-global env on the per-session path. Thread launch command, compositor, input-backend, and session env into the per-session `VirtualDisplay`/`SessionContext` (as GameStream already does via `set_launch_command`) and pass them as explicit args to backend/injector open calls. At minimum serialize all `set_var` writes + dependent backend-open under one mutex, or force `max_concurrent=1` while the auto env-retargeting state machine is active.
|
||
|
||
---
|
||
|
||
### 8. [Low] `web-password` written world-readable then `icacls`'d — brief TOCTOU disclosure — *Confirmed*
|
||
|
||
- **Surface:** Secret-file permissions (Windows install).
|
||
- **Refs:** `windows/install.rs:273-290`.
|
||
- **Threat actor:** Local unprivileged user (#4). Windows, install/upgrade time only.
|
||
- **Mechanism:** `set_web_password` writes the cleartext `PUNKTFUNK_UI_PASSWORD=<pw>` via `std::fs::write` (creating the file at the inherited Users-readable `%ProgramData%` ACL) and only *afterward* strips inheritance with `icacls`. Between the write and the `icacls` child-process completion (a full process spawn = a race-winnable window) the web-console login password is readable by any local user.
|
||
- **Attack scenario:** A local user polling `%ProgramData%\punktfunk` during a fresh install reads `web-password` before `icacls` applies, obtaining the web-console login credential.
|
||
- **Existing mitigations:** Window is fresh-install-only (on upgrade the existing file's locked DACL is preserved across a truncating write, so no window reopens — the "upgrade rewrites the password" sub-claim does not hold); install is operator-initiated and one-time; `icacls` locks immediately after; impact limited to web-console access.
|
||
- **Verifier adjudication:** One verifier **confirmed low**; the other adjusted to **info**, noting the write-then-`icacls` pattern is the established Windows secret pattern (used by `write_secret_file` for far higher-value secrets), so the "anomalously non-atomic" framing is overstated and this is the lowest-value secret affected. Carried at **low**.
|
||
- **Recommendation:** Create the file with a restrictive DACL atomically (`CreateFile` with a SECURITY_DESCRIPTOR, or write to a per-process temp under an already-locked dir then rename), or write empty + `icacls` before writing the secret bytes.
|
||
|
||
---
|
||
|
||
### 9. [Low] Unpaired LAN peer can burn the operator's single-use pairing window — *Confirmed*
|
||
|
||
- **Surface:** Native SPAKE2 pairing.
|
||
- **Refs:** `punktfunk1.rs:459` (`np.disarm()` before proof verification), `punktfunk1.rs:438` (`pake.finish` accepts a wrong-PIN message), `punktfunk1.rs:517-531` (cooldown / `current_pin()`), `native_pairing.rs:216-218` (`disarm`), `quic.rs:1581` (`AcceptAnyClientCert`).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1). Native path, while pairing is armed.
|
||
- **Mechanism:** The single-use design disarms the PIN on *any* well-formed pairing attempt, **before** verifying the guess (the disarm-before-verify behavior is exactly prior-fix #2, which gives the single-online-guess guarantee). `pake.finish()` does not reject a wrong-PIN `spake_a` (only malformed messages), so an unpaired peer with a self-signed cert and a SPAKE2 message built from any random PIN guess reaches `disarm` and consumes the window without knowing the PIN.
|
||
- **Attack scenario:** Operator arms pairing; an attacker polling the QUIC port every ~2 s (the `PAIRING_COOLDOWN`) lands an attempt inside the ~120 s armed window; the host disarms. The legitimate device then submits the real PIN and is told "pairing not armed." Repeat indefinitely.
|
||
- **Existing mitigations:** Availability-only (1/10000 chance a blind guess actually pairs — the documented single online guess). The attack only works *while a window is armed* (outside it, `current_pin()` is `None` and the handshake bails before touching disarm), so it cannot permanently disable pairing — it races an open window. **The delegated-approval flow (knock → console approve) is structurally immune** and remains usable on hostile LANs.
|
||
- **Verifier adjudication:** One verifier **confirmed low**; the other adjusted to **info** as a self-acknowledged, in-code-documented design tradeoff with an immune fallback. Carried at **low**.
|
||
- **Recommendation:** Prefer the delegated-approval flow on hostile LANs (already immune). Document that PIN arming should be brief. If retaining PIN arming, consider only consuming the window on a key-confirmation match when the failure is observable (trading some brute-force resistance for availability).
|
||
|
||
---
|
||
|
||
### 10. [Low] ENet control flood → unbounded per-packet warn-log spam — *Confirmed*
|
||
|
||
- **Surface:** GameStream ENet control plane.
|
||
- **Refs:** `gamestream/control.rs:84`/`161` (`on_receive`), `control.rs:186` (per-packet `tracing::warn!`, no throttle), `control.rs:316`/`347-378` (decrypt + scheme sweep), `control.rs:79` (`detected` reset on Disconnect).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1). Requires `--gamestream` and an active paired session.
|
||
- **Mechanism:** The ENet control host (UDP 47999, `peer_limit=4`) accepts unauthenticated connections. Once a paired client has launched (global `gcm_key` set), any `0x0001`-prefixed packet with a ≥16-byte payload that fails to authenticate emits one `tracing::warn` per packet with **no rate limit or sampling**. The full ~72-candidate GCM scheme-sweep runs only while `detected` is `None` (a transient window; the attacker can reset it via its own Disconnect but steady state is one GCM op + one warn per packet).
|
||
- **Attack scenario:** With a paired session active, an attacker ENet-connects and floods junk `0x0001` packets → unbounded warn-log lines (disk/observability pressure) + intermittent CPU.
|
||
- **Existing mitigations:** `peer_limit=4`; the expensive sweep is `detected`-gated; AES-GCM open on tiny buffers is microseconds; input injection itself stays cryptographically gated on the HTTPS-delivered `gcm_key` (no forgery). Opt-in, trusted-LAN.
|
||
- **Verifier adjudication:** **Confirmed**; the "per-packet GCM brute-force" framing is largely neutralized by the `detected` fast-path, but the **unthrottled per-packet warn log** is genuinely unmitigated. Low severity (DoS/observability only, no injection or memory unsafety).
|
||
- **Recommendation:** Throttle/aggregate the "GCM decrypt failed" warning (sampled, not per-packet) and drop a peer after N consecutive auth failures; optionally skip the scheme-sweep for a peer that has produced no authenticating packet.
|
||
|
||
---
|
||
|
||
### 11. [Low→Info] SYSTEM `host.log` opened with predictable name in a Users-writable directory — *Partial*
|
||
|
||
- **Surface:** Windows service / logging.
|
||
- **Refs:** `windows/service.rs:121-125` (logs dir via plain `create_dir_all`), `service.rs:574-602` (`open_log_handle`, `OPEN_ALWAYS`, append-only, inheritable, `FILE_SHARE_READ|WRITE`).
|
||
- **Threat actor:** Local unprivileged user (#4). Windows.
|
||
- **Mechanism:** The SYSTEM service opens `%ProgramData%\punktfunk\logs\host.log` and redirects the host child's stdout/stderr to it. The logs dir lives under the non-DACL-locked config tree (Finding 3). A local user able to create files there could pre-create `host.log` as an NTFS hardlink to an attacker-chosen target, causing SYSTEM's appends to land on that target.
|
||
- **Impact:** Limited integrity: SYSTEM appends *attacker-uncontrolled* log text (append-only handle — no truncation, no chosen-offset writes) to an attacker-chosen file. No content control → no realistic code-exec path; a log-tamper/nuisance/DoS primitive at most.
|
||
- **Verifier adjudication:** Both verifiers found the redirect-*target* control hinges on a non-admin holding `FILE_ADD_FILE` on a SYSTEM-created subdir, which the default `%ProgramData%` ACL does **not** grant (Users get create-subfolder, not create-file). The only residual is the same **pre-install directory-squatting** edge as Finding 3, and even then the writes are append-only uncontrolled text. One verifier **partial/low**, one **partial/info**. Effectively a sub-case of Finding 3.
|
||
- **Recommendation:** Fixing Finding 3 (DACL-lock the config/logs dir to SYSTEM+Administrators) fixes this. Optionally open the log rejecting reparse points / create the dir with a restrictive DACL before first write.
|
||
|
||
---
|
||
|
||
### 12. [Low→Info] Legacy GameStream pairing has no rate-limit and parks unbounded 300 s waiters — *Partial*
|
||
|
||
- **Surface:** GameStream pairing.
|
||
- **Refs:** `gamestream/pairing.rs:102-127` (`getservercert` parks `pin.take(300s)`), `pairing.rs:50-60` (`WaiterGuard`), `nvhttp.rs:215-244` (unauthenticated `/pair` route, no rate limit / connection cap).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1). Requires `--gamestream`.
|
||
- **Mechanism:** `/pair?phrase=getservercert` is reachable pre-auth with an attacker-chosen `uniqueid` and no per-IP/global rate limit; each parks a tokio task for up to 300 s and keeps `awaiting_pin` asserted. The HTTP server has no connection cap (bare `axum_server::bind`).
|
||
- **Verifier adjudication:** Both verifiers **confirmed the no-rate-limit/parked-waiter core but refuted the alarming "unbounded never-evicted HashMap"** sub-claim — the `sessions` insert is downstream of a successful `pin.take()`, which requires an operator-delivered PIN, so the map grows at most one entry per PIN submission (not attacker-driven). The residual is a bounded (300 s self-heal, cheap tasks), opt-in slow-loris + `awaiting_pin` nuisance on a surface already covered by accepted-risk #5/#9, plus a minor enlargement of the Finding-9-class PIN race. Both adjusted to **info**.
|
||
- **Recommendation:** Add a per-source-IP / concurrent-handshake cap on pairing attempts and evict the per-`uniqueid` session on success/timeout (not only on failure).
|
||
|
||
---
|
||
|
||
### 13. [Info] Pending-approval queue floodable by a LAN cert flood — *Confirmed*
|
||
|
||
- **Surface:** Native pairing / delegated-approval queue.
|
||
- **Refs:** `native_pairing.rs:336-357` (`note_pending`, `PENDING_CAP=32` eviction of least-recently-active), `native_pairing.rs:81-83` (cap + 10-min TTL), `punktfunk1.rs:566` (called per unpaired knock, no per-source rate limit).
|
||
- **Threat actor:** Malicious network client, **pre-auth** (#1).
|
||
- **Mechanism:** `note_pending` is called for every unpaired-but-identified knock with no per-source rate limit; past 32 entries the least-recently-active is evicted. An attacker minting >32 distinct self-signed certs can churn the queue, potentially evicting a quiet legitimate knock before the operator approves it.
|
||
- **Verifier adjudication:** One **confirmed info**, one **refuted** — the in-place refresh resets `requested_at` on every same-fingerprint re-knock, so an actively-retrying legitimate device is structurally non-evictable; only a one-shot knock-and-wait device is at risk and it recovers instantly by re-knocking. Each junk slot costs a full QUIC handshake; no trust-store/PIN/key impact. Carried at **info** (transient self-healing availability nuisance on the convenience queue only).
|
||
- **Recommendation:** Optionally cap pending entries per source IP/subnet, or surface a "pending overflow" indicator. Low priority.
|
||
|
||
---
|
||
|
||
## Prior-fix verification (#1–#12)
|
||
|
||
- **#1 — HIGH (secret-file perms 0600/0700 Unix; SYSTEM+Admins DACL Windows): PRESENT but INCOMPLETE — regressed for two newer files.** The core helpers `create_private_dir` (0700 Unix) and `write_secret_file`/`restrict_to_system_admins` (Unix 0600 + Windows `icacls` SYSTEM/Admins/OWNER) are correct and used for `key.pem`, `cert.pem`, GameStream `paired.json`, native `punktfunk1-paired.json`, and `web-password` (all atomic temp+rename, no world-readable window, never logged). **Gaps:** the mgmt-token writer (`mgmt_token.rs:write_token`) hardens only `cfg(unix)` and never applies the Windows DACL (**Finding 2**); `host.env` is written with a bare `std::fs::write` and the Windows config *directory* is never DACL-locked (**Finding 3**); `web-password` has a brief write-then-`icacls` TOCTOU window (**Finding 8**). Non-secret files (`uniqueid`, `library.json`, art cache, stats captures) carry no key material — acceptable.
|
||
- **#2 — HIGH (native SPAKE2 PIN single-use): VERIFIED INTACT.** `np.disarm()` runs unconditionally before reading the client proof (`punktfunk1.rs:459`); a malformed `spake_a` errors earlier but makes no guess. The global `PAIRING_COOLDOWN` (2 s) + per-attempt `current_pin()` close the concurrency TOCTOU; CSPRNG PIN; CLI arm-at-start is also consumed. No path leaves a static reusable PIN. (The single-use design's only side effect is the availability edge of **Finding 9**.) *Caveat:* the **legacy GameStream** `PinGate` is a separate mechanism — `PinGate::take()` consumes the PIN and the mgmt path guards `awaiting_pin()`, but the nvhttp `/pin` path does **not** guard and is unauthenticated (**Finding 1**).
|
||
- **#3 — MED (RTSP packetSize clamp + saturating packetizer): VERIFIED PRESENT.** `rtsp.rs:330-339` rejects packetSize outside `64..=2048`; `video.rs:63` clamps `payload_per_shard` so all divisors are ≥1 (regression test `degenerate_packet_size_does_not_panic`).
|
||
- **#4 — LOW (mgmt mTLS cert restricted to read-only allowlist): VERIFIED COMPLETE.** `cert_may_access` (`mgmt.rs:514-528`) is GET-only over an exact-path set excluding every state-changing/pairing/stats route; all `/api/v1` routes share `route_layer(require_auth)`; cert branch additionally requires `native.is_paired(fp)`. No streaming cert can read the PIN, self-approve, mutate the library, or reach `/stats/*`. Not regressed by any newly-added route.
|
||
- **#5 — LOW ACCEPTED (legacy control-stream GCM nonce reuse): UNCHANGED.** Still legacy/Moonlight-compat (`control.rs:108-117`); not reachable on the default `serve` path. Not re-flagged.
|
||
- **#6 — LOW (RTSP header/Content-Length caps + read timeout + connection cap): VERIFIED PRESENT.** `MAX_RTSP_CONNS=8`, `RTSP_READ_TIMEOUT=15s`, 16K header / 64K body / 128K message caps enforced; `ConnGuard` releases the slot on panic.
|
||
- **#7 — LOW PARTIAL (per-session launch command; native path used a process-global env): STILL UNRESOLVED and now REGRESSED IN IMPACT.** The native path still does `std::env::set_var("PUNKTFUNK_GAMESCOPE_APP", cmd)` and the gamescope backend reads that global; the in-code "ONE-session-at-a-time" justification is invalidated by `DEFAULT_MAX_CONCURRENT=4`. The GameStream/Windows path correctly threads launch into a per-session `SessionContext`. This is now **Finding 7** (generalized to the whole env-retargeting state machine + a `set_var`/`getenv` data race).
|
||
- **#8 — INFO (GameStream phase-4 hash compare constant-time): VERIFIED PRESENT.** `pairing.rs:228` uses `crypto::ct_eq`, a proper no-early-exit fold; `hash_ok` and `sig_ok` are both computed before branching. Mgmt `token_eq` similarly SHA-256-hashes both sides.
|
||
- **#9 — INFO ACCEPTED (/pair over plain HTTP): UNCHANGED** as a transport matter. **Note:** the *unauthenticated `/pin` self-delivery* (Finding 1) is a distinct, newly-surfaced defect, **not** subsumed by #9.
|
||
- **#10 — INFO (fixed ALPN `pkf1` on QUIC): VERIFIED PRESENT.**
|
||
- **#11 — INFO (FEC reconstruct failure = drop not fatal): VERIFIED PRESENT.** Host encode uses `encode(...).unwrap_or_default()`; audio returns `None` to skip a block; no fatal path.
|
||
- **#12 — LOW DEFERRED (web `NODE_TLS_REJECT_UNAUTHORIZED`): out of host scope, not examined.**
|
||
|
||
## Refuted / investigated — not vulnerabilities
|
||
|
||
- **PinGate PIN not bound to uniqueid/cert (confused-deputy PIN theft) — *refuted.*** The global PIN-slot race is real (enables a pairing DoS, folded into Finding 9's class), but the escalation is cryptographically impossible: in GameStream the PIN is *generated and displayed by the Moonlight client* and the host never echoes it, so a racing attacker consumes the PIN-submission *event* but never learns the PIN *value*; without it the phase-2/4 hash + RSA checks fail closed. No paired identity gained.
|
||
- **Attacker-chosen device name in the approval queue (trusted-device impersonation) — *refuted.*** The unpaired knock is hard-rejected; the fingerprint (the value actually pinned) is displayed alongside the sanitized name, and bidi/control/homoglyph chars are stripped. Approval requires a bearer-authenticated human; "approving on the label without reading the fingerprint" is social engineering inherent to any human-in-the-loop pairing, with the standard mitigation already present.
|
||
- **Lutris cover-art slug path traversal — *refuted.*** The `..`-joined read is real, but `slug` originates from the host user's own `~/.local/share/lutris/pga.db` (a same-user local file), not controllable by any in-scope network/MITM/local-unpriv adversary; the disclosure recipient is an already-paired client with strictly greater authority, and the read is `.jpg`-only, ≤1 MiB. Charset-validating the slug is worthwhile defense-in-depth.
|
||
- **Privileged install invokes system tools by bare name (PATH/CWD hijack) — *refuted.*** Premise is wrong for the Rust toolchain: `std::process::Command` resolves the executable itself, searches `System32` *before* the CWD, and never searches the spawning process's directory. All cited tools are System32 binaries, so a planted CWD copy loses. Using absolute `%SystemRoot%\System32\…` paths is reasonable consistency hardening but addresses no reachable threat.
|
||
- **`uniqueid`/mgmt-token create the config dir with `create_dir_all` (brief 0755) — *refuted.*** Every secret file is written 0600/DACL-locked regardless of directory mode; the only non-secret file (`uniqueid`) is a public serverinfo identifier; on Linux the dir is under the owning user's per-user home; `create_private_dir` later tightens it to 0700. Code-consistency cleanup, no disclosure.
|
||
- **Unbounded on-disk stats capture files — *refuted.*** Every `/stats/*` route is bearer-token-gated (excluded from the cert allowlist); the captures dir is 0700; the file id is host-generated. No pre-auth, post-auth, MITM, or local-unpriv path can create captures — only the trusted operator over their own disk. Pruning/streamed `list()` parsing is a reasonable operational improvement, not a security fix.
|
||
|
||
## Cross-cutting themes
|
||
|
||
1. **GameStream/Moonlight compatibility is the soft underbelly.** Both pre-auth bypasses (Findings 1, 4) and the control-plane DoS (Finding 10) live exclusively on the opt-in `--gamestream` surface, whose authz model is weaker by design (accepted-risk #5/#9). The native punktfunk/1 plane is markedly stronger. The two genuinely new pre-auth issues — unauthenticated `/pin` self-pairing and the ungated RTSP media plane — are *bypasses of GameStream's own `peer_is_paired` boundary*, not inherent-protocol weaknesses, and are fixable without breaking stock-Moonlight compatibility.
|
||
2. **Prior-fix #1 hardened secret *files* but not the Windows config *directory* or two files added since.** Findings 2, 3, 8, 11 all trace to the `%ProgramData%\punktfunk` ACL gap plus the bespoke `write_token`/`std::fs::write` paths that bypass `write_secret_file`. A single remediation — DACL-lock the config directory and route *all* config writes through `write_secret_file` — closes most of the Windows local-privilege surface.
|
||
3. **Concurrency outgrew single-session assumptions.** Finding 7 (and the regressed prior-fix #7) is the codebase shipping default `max_concurrent=4` while per-session state still uses process-global `std::env` mutation written under a one-session invariant. The `SessionContext`/`set_launch_command` pattern already used on the Windows/GameStream path is the correct fix to generalize.
|
||
4. **Local IPC and temp-file trust.** The Windows gamepad/IDD shared sections (`Everyone:GENERIC_ALL`, Finding 5) and the Linux gamescope EIS `/tmp` relay (Finding 6) both trust a local channel that a second unprivileged account can read/write. Scope DACLs to the consuming principal and move relays into owner-private runtime dirs.
|
||
|
||
## Security controls done right (positives)
|
||
|
||
- **Native SPAKE2 pairing is well-hardened:** single-use disarm-before-verify, global cooldown, atomic+rollback persist, fail-closed load, CSPRNG PIN, device-name sanitization (C0/C1 + bidi/format stripped, 64-char cap) at every sink, with regression tests. No path lets an unpaired peer self-approve, read the PIN, or poison the trust store.
|
||
- **Post-pair cert-pinning is sound:** the TLS layer verifies the `CertificateVerify` signature (key ownership) even though it "accepts any" cert at handshake, and `peer_is_paired` pins SHA-256(DER) against the saved cert — a stolen public cert cannot impersonate a paired client.
|
||
- **Management authz is solid:** every `/api/v1` route gated (even on loopback), `run` refuses to start without a token, loopback-default bind, constant-time (SHA-256-hashed) token compare, 256-bit token entropy, no cookie/CSRF surface, and a correct read-only-cert vs. bearer-mutation split.
|
||
- **The new library/launch surface is strong against the network adversary:** client ids resolve against the host's own scanned catalog (never client-supplied launch strings), Steam appids are digit-validated, Heroic/Epic/AUMID values charset-validated, all non-operator spawns are argv-based with no shell, and the only `cmd.exe /c`/`sh -c` sinks consume operator-typed input only. No SSRF in the cover-art warmer (fixed trusted hosts, ids in the path component only). XML/JSON/VDF parsers are entity-expansion-safe.
|
||
- **Wire parsers are memory-safe:** RTSP has connection caps, read timeouts, header/body/message caps, and clamps every attacker-controlled numeric; the video packetizer is structurally panic-proof; input/gamepad decoders are fully `.get()`-bounded with `idx < MAX_PADS` checks; DualSense/DS4 output-report parsers bounds-check before indexed reads.
|
||
- **The stats-capture surface is clean:** bearer-only routes, host-generated path-safe ids with traversal rejection (tested), 0700 captures dir, bounded samples, lock-serialized hot-path feed, and host-derived (non-free-form) metadata fields.
|
||
- **Session/cross-user isolation holds:** the Desktop↔Game follow watcher and `detect_active_session` filter `/proc` strictly by the host's own uid, so a session can never follow or expose a different user's compositor; per-session virtual-output/encoder teardown is sound RAII (no monitor/FD/zombie leaks); `--max-concurrent` genuinely caps concurrency.
|
||
- **Windows service launch hygiene:** fully-quoted `current_exe` binPath with fixed args (no unquoted-service-path), correct token scoping (drops to the user token for store launchers/WGC, retains SYSTEM only for our own streamer), anonymous inherited pipes for the host↔helper channel, and no command line built from network input.
|
||
|
||
---
|
||
|
||
## Supplement (2026-06-28, follow-up pass 2 — completed surfaces + coverage-critic gaps)
|
||
|
||
### (a) Summary
|
||
|
||
This pass closes the two finders that failed in the main audit (native protocol; unsafe FFI — here split into control-plane, data-plane, encode/capture, and driver-IPC) and the three coverage-critic gaps (mic/Opus → virtual mic + cross-session isolation; `main.rs` default-security posture + dependency RUSTSEC; cover-art outbound egress/SSRF). The headline answers: **the native control plane is fail-closed for unpaired peers at the application layer** — `serve_session` rejects anonymous/unpaired clients before any session machinery (`punktfunk1.rs:544-573`) — **but the QUIC *transport* underneath is not**, and it is the only genuinely pre-auth crown-jewel-adjacent exposure found here: `quinn-proto 0.11.14` (RUSTSEC-2026-0185, CVSS 7.5 unbounded out-of-order reassembly) is reachable by any unpaired peer who completes the 1-RTT handshake with a throwaway cert *before* the pairing gate runs → remote memory-exhaustion DoS of the always-on default listener. **Client geometry is well bounds-checked** (W/H caps applied on Hello, Reconfigure, and ANNOUNCE; Opus mic buffer math is exact; gamepad/touchpad indices clamped) with one consistent gap: the **refresh/fps lower bound is unvalidated on the initial Hello path** (the Reconfigure path guards it), yielding at worst a self-inflicted single-session divide-by-zero panic. **Cover-art egress is SSRF-safe against every in-scope adversary** (hardcoded hosts, id only in the path segment, TLS verification on); the only residual is an out-of-scope supply-chain redirect-follow. **The rsa 0.9 Marvin oracle is not practically reachable** — it is a signing path (not the classic PKCS#1v1.5 decryption oracle), on the opt-in trusted-LAN-only GameStream plane. The mic/Opus surface adds one real cross-session defect: a malformed Opus frame tears down the single host-lifetime virtual mic shared by all concurrent sessions. The driver-IPC surface is **memory-safe and clean** (the only weakness is the already-reported world-writable section ACL).
|
||
|
||
### (b) Confirmed and partial findings
|
||
|
||
#### S1 — Pre-auth remote memory exhaustion via vulnerable `quinn-proto 0.11.14` on the always-on native QUIC control plane (RUSTSEC-2026-0185) — **CONFIRMED, severity HIGH**
|
||
- **Surface:** cli-posture-deps / native QUIC transport. **Files:** `Cargo.lock` (quinn-proto 0.11.14, line ~2966), `crates/punktfunk-core/src/quic.rs:1540-1589,1580-1581,1723-1740`, `crates/punktfunk-host/src/punktfunk1.rs:176-181,503`.
|
||
- **Threat actor / auth:** malicious network client, **pre-auth** (unpaired, unauthenticated).
|
||
- **Mechanism:** `serve` (the secure default) always builds the native QUIC listener bound to `0.0.0.0:9777`. The rustls `ServerConfig` uses `AcceptAnyClientCert` and defers *all* identity/pairing verification to a post-handshake app-layer fingerprint check. An unpaired peer therefore presents any self-signed cert, completes the QUIC 1-RTT handshake, and reaches `quinn-proto`'s stream-reassembly path **before** the `--require-pairing` gate. RUSTSEC-2026-0185: unbounded out-of-order STREAM-frame buffering → remote memory exhaustion.
|
||
- **Scenario:** attacker on the LAN sends a ClientHello, finishes the handshake with a throwaway cert, opens a stream, floods out-of-order STREAM frames with large gaps; the privileged host buffers unboundedly → OOM, killing streaming for all paired clients and possibly the box.
|
||
- **Existing mitigations:** `--max-concurrent` bounds session *count* but not per-connection reassembly memory; the pairing gate runs after the vulnerable transport layer; `stream_transport()` sets only idle-timeout/keep-alive, not receive-window limits. None neutralize this.
|
||
- **Recommendation:** `cargo update -p quinn-proto --precise 0.11.15` (or bump `quinn`), and wire `cargo audit` into CI as a failing gate on the QUIC path.
|
||
- **Verifiers:** both confirmed, **adjusted_severity HIGH** (availability-only — no key/trust-store impact — so high, not critical). Exploit path corroborated end-to-end: `main.rs:503` always-on default → `server_with_identity` → `AcceptAnyClientCert` accepts any cert → handshake reaches quinn-proto reassembly pre-pairing.
|
||
|
||
#### S2 — Malformed client Opus mic frame tears down the shared host-lifetime virtual mic (cross-session DoS) — **CONFIRMED, severity LOW–MEDIUM**
|
||
- **Surface:** audio-mic-decode. **Files:** `crates/punktfunk-host/src/punktfunk1.rs:1231-1280` (esp. 1266-1277), `:221,:292/:300`; `crates/punktfunk-core/src/quic.rs:1210`.
|
||
- **Threat actor / auth:** malicious paired client, **post-auth**.
|
||
- **Mechanism:** `mic_service_thread` treats *any* `opus::Decoder::decode_float` error as a backend failure: it sets `mic=None; decoder=None; last_failed=now`, tearing down the PipeWire/WASAPI virtual mic and forcing a 2s `INJECTOR_REOPEN_BACKOFF`. The Opus payload is raw attacker bytes (`decode_mic_datagram` checks only `len>=13` and forwards `b[13..]` verbatim), and libopus returns `OPUS_INVALID_PACKET` on a malformed TOC, so a single crafted ≥14-byte datagram triggers it. Critically, the `MicService` is **one host-lifetime resource shared by every concurrent session** (created once in `serve()`, sender cloned per session).
|
||
- **Scenario:** paired client #2 (a second concurrent session) sends one garbage Opus frame every ~2s; the shared mic thread repeatedly drops the virtual mic and re-enters backoff, keeping the microphone unavailable for session #1's recording/voice-chat app — a **cross-session** denial of an optional feature beyond the offender's own tier.
|
||
- **Existing mitigations:** pairing-gated; 2s backoff bounds reopen churn; DTX/empty frames skipped; no memory blow-up. None prevent the cross-session denial because there is no per-session decoder/mic isolation.
|
||
- **Recommendation:** treat a codec decode error as a per-frame drop (rate-limited log), keeping decoder+mic open; only tear down on an actual backend `push` error; reset (not destroy) decoder state; ideally use a per-session decoder.
|
||
- **Verifiers:** both confirmed; **adjusted_severity split MEDIUM / LOW** — medium because a low-effort paired client denies an honest concurrent session's mic (genuine new authority via the shared resource); low because the impact is confined to one optional feature, churn-bounded, no crash/disclosure/exec, and all paired clients already share one desktop at a high mutual-trust tier. Net: treat as **LOW–MEDIUM**, fix is cheap and warranted.
|
||
|
||
#### S3 — Unbounded held-button/held-key tracking `Vec` grows on attacker-chosen input codes (per-session DoS) — **CONFIRMED, severity LOW**
|
||
- **Surface:** native-data-plane. **Files:** `crates/punktfunk-host/src/punktfunk1.rs:1457-1483` (esp. 1476-1483); `crates/punktfunk-core/src/input.rs:136-149`.
|
||
- **Threat actor / auth:** malicious paired client, **post-auth**.
|
||
- **Mechanism:** every `MouseButtonDown`/`KeyDown` whose 32-bit `ev.code` (read straight off the wire at `input.rs:144`, no range/validity check) is not already present is pushed into the per-session `held_buttons`/`held_keys` `Vec`, with no cap and a linear `Vec::contains` presence test (O(n) per event, O(n²) over a run). Entries are removed only by a matching Up. The upstream mpsc is also unbounded with no per-packet throttle.
|
||
- **Scenario:** paired client floods `MouseButtonDown`/`KeyDown` with monotonically increasing `code`s and never sends Up → the `Vec` grows unbounded and the quadratic scan spikes the session's input-thread CPU for the session lifetime.
|
||
- **Existing mitigations:** per-session `Vec`s dropped on disconnect; input injection is in-scope-by-design (the *only* new harm is the unbounded *tracking* state); QUIC intake is receive-buffer bounded.
|
||
- **Recommendation:** bound the held-state sets with a `HashSet` keyed by `code` (removes the O(n²) scan) and/or reject codes outside valid button/key ranges before tracking; cap the number of distinct held codes.
|
||
- **Verifiers:** both confirmed, **adjusted_severity LOW** — self-confined to one session thread, no host crash, inverted amplification (wire bytes > memory), but a real unnecessary unbounded-growth defect.
|
||
|
||
#### S4 — Unbounded read of local launcher caches (Epic `catcache.bin` / `.item` manifests) — memory-exhaustion DoS — **CONFIRMED, severity LOW**
|
||
- **Surface:** cover-art-egress / library enumeration. **Files:** `crates/punktfunk-host/src/library.rs:657-665` (esp. `std::fs::read` at ~660 + base64 decode ~663), `:580` (`read_to_string`).
|
||
- **Threat actor / auth:** local unprivileged user (Windows host), **post-auth N/A** (local).
|
||
- **Mechanism:** `epic_art_index` reads the entire `%ProgramData%\Epic\EpicGamesLauncher\Data\Catalog\catcache.bin` with **no size cap**, then base64-decodes it (a second ~0.75× allocation), then `serde_json` parses — stacked unbounded allocations in the LocalSystem host. Each `.item` manifest is likewise read whole. Default ProgramData ACLs commonly let a standard user create/replace files in app subfolders (Epic itself grants Users modify so its user-mode launcher can rewrite the cache).
|
||
- **Scenario:** local user plants a multi-GB `catcache.bin`; the next library enumeration (mgmt list / GameStream serverinfo-applist / art warmer `all_games()`) loads it plus its decoded copy into the privileged host → OOM.
|
||
- **Existing mitigations:** best-effort (failures return empty map, no crash); triggered per-enumeration, not continuously; Windows-only. Notably the Linux `lutris_image` reader (`library.rs:372-377`) **already caps at 1 MiB** — the pattern is known and simply not applied here.
|
||
- **Recommendation:** `fs::metadata` size check or a `take()`-limited reader (a few MB for `catcache.bin`, tens of KB per `.item`) before read/decode; skip oversize files.
|
||
- **Verifiers:** both confirmed, **adjusted_severity LOW** — DoS only, ACL-precondition reduces exploitability but not the verdict; the author's own Linux cap proves the omission.
|
||
|
||
#### S5 — Client refresh/fps lower bound not validated before encoder open (Hello path; folded across two finders) — **PARTIAL, severity LOW→INFO**
|
||
- **Surface:** native-control-plane + unsafe-encode-capture (these two finders are the **same defect** at different depths; reported once here). **Files:** `crates/punktfunk-host/src/encode.rs:195-211` (`validate_dimensions`), `crates/punktfunk-host/src/punktfunk1.rs:574-579,804,3659-3663`, `crates/punktfunk-host/src/encode/linux/mod.rs:247-248,474`, `crates/punktfunk-host/src/encode/linux/vaapi.rs:98,184`.
|
||
- **Threat actor / auth:** malicious paired client, **post-auth** (pre-auth only on opt-in `--open`/`--allow-tofu`).
|
||
- **Mechanism:** `validate_dimensions` caps W/H but ignores refresh. The mid-stream **Reconfigure** path explicitly checks `req.mode.refresh_hz > 0` (`punktfunk1.rs:804`) — proving the invariant is known — but the **initial Hello** path does not. On the common Linux backends (gamescope/wlroots/mutter) `preferred_mode` echoes the requested refresh, so `effective_hz`'s `.filter(|hz| hz>0).unwrap_or(mode.refresh_hz)` collapses a requested `refresh_hz=0` back to 0, reaching `open_video(fps=0)` → `time_base = Rational(1,0)` and the unchecked `pts * 1e9 / self.fps` divide at `encode/linux/mod.rs:474` (and `vaapi.rs:184`).
|
||
- **Scenario:** a paired client sends `Hello{mode: WxHx0}`; on a Mutter/wlroots/gamescope host either `avcodec_open2` rejects the `1/0` time_base (clean Err) or the first packet triggers a divide-by-zero panic on the encode thread.
|
||
- **Impact / mitigations:** at worst a **single-session-thread panic** isolated by `spawn_blocking`/`panic=unwind` (surfaces as a JoinError at `punktfunk1.rs:1092-1094`; the persistent listener and sibling sessions survive). KWin reports a real achieved Hz and dodges it. The **GameStream half is refuted**: `rtsp.rs:340-342` floors `maxFPS` with `.filter(|&f| f>0).unwrap_or(60)`, so `cfg.fps` is never 0.
|
||
- **Recommendation:** fold a refresh lower-bound (`>0`, ideally clamp `1..=480`) into `validate_dimensions` so Hello and Reconfigure enforce the same invariant; defensively use `self.fps.max(1)` at the two division sites.
|
||
- **Verifiers:** all four lenses PARTIAL; **adjusted_severity INFO–LOW** — a real validation asymmetry and reachable divide-by-zero, but the outcome is a self-inflicted teardown of the attacker's *own* isolated session granting no new authority (post-auth) or reducing to the already-accepted stream-slot DoS (on opt-in `--open` hosts). Worth the trivial fix; not a boundary crossing.
|
||
|
||
#### S6 — Unbounded mpsc into the host-lifetime shared `MicService` (0xCB) — **PARTIAL (leaning info), severity LOW→INFO**
|
||
- **Surface:** native-data-plane / audio. **Files:** `crates/punktfunk-host/src/punktfunk1.rs:905-911,1200,1231-1280`; sinks `audio/linux/mod.rs:151-153`, `audio/windows/wasapi_mic.rs:107-120`.
|
||
- **Threat actor / auth:** malicious paired client, **post-auth**.
|
||
- **Mechanism (as filed):** each session forwards every 0xCB frame into an unbounded host-lifetime `std::sync::mpsc` shared across all sessions, with no backpressure/cap; the single consumer does an Opus decode + virtual-mic push per iteration.
|
||
- **Verifier correction:** the filed DoS mechanism — "the push blocks on the audio backend, so the queue grows without bound" — is **factually wrong**. Both `VirtualMic::push` impls are non-blocking and self-bounded: Linux uses `try_send` (drops when behind); Windows takes a quick mutex with a drop-oldest `MAX_QUEUE_BYTES` cap. The consumer is therefore CPU-throughput-limited (decode-only), runs on its own thread, and never stalls; the producer is QUIC-receive-rate bounded doing comparable per-item work. Items are only the ~sub-1KB Opus payload.
|
||
- **Residual:** a genuine but minor robustness gap — an unbounded shared channel with no explicit cap/rate-limit; under a sustained near-line-rate flood exceeding decode throughput, a small producer>consumer gap could accumulate.
|
||
- **Recommendation:** use a bounded (drop-oldest) channel for the mic forward, or rate-limit/coalesce per-session before the shared service.
|
||
- **Verifiers:** both PARTIAL, **adjusted_severity INFO–LOW** — structural claim holds, stated stall mechanism refuted by the non-blocking sinks.
|
||
|
||
#### S7 — GameStream RSA pairing uses `rsa 0.9` (RUSTSEC-2023-0071 Marvin timing side-channel) — **PARTIAL (leaning info), severity LOW→INFO**
|
||
- **Surface:** cli-posture-deps. **Files:** `Cargo.toml` (rsa 0.9.10), `crates/punktfunk-host/src/gamestream/cert.rs:54-55`, `crates/punktfunk-host/src/gamestream/pairing.rs:200`.
|
||
- **Threat actor / auth:** network adversary on the GameStream pairing flow, **pre-auth** (the pairing flow itself; the consent bypass is already tracked in the main audit).
|
||
- **Mechanism:** the host's persistent RSA-2048 identity (the trust root: pinned TLS cert + pairing signer) is loaded into a PKCS1v15 `SigningKey` and used to `sign(&serversecret)` during the unauthenticated nvhttp pairing ceremony. `rsa 0.9.10` carries RUSTSEC-2023-0071 (variable-time private-key op, no fixed upstream release), so signing-response timing is data-dependent on the secret key. Recovery would defeat client cert-pinning (host impersonation).
|
||
- **Existing mitigations:** GameStream is **off by default and documented trusted-LAN-only** (#5/#9 inherent caveat); the native plane uses Ed25519/SPAKE2 and is unaffected. Crucially this is the **signing** path, not the PKCS#1v1.5 **decryption** oracle Marvin classically targets, and `serversecret` is host-generated random (not attacker-chosen) — so a remote network-timed RSA-2048 key recovery over a jittery LAN is theoretical, requiring enormous high-precision sampling.
|
||
- **Recommendation:** track the advisory; when a blinded/constant-time `rsa` release lands, upgrade; consider migrating the GameStream identity to ECDSA/Ed25519; keep GameStream gated off by default.
|
||
- **Verifiers:** both PARTIAL, **adjusted_severity INFO–LOW** — claim technically accurate and no code-level fix exists upstream, but the off-by-default posture, signing-not-decryption distinction, and lack of any demonstrated practical remote key recovery reduce this to a transitive-advisory exposure.
|
||
|
||
### (c) Refuted / not vulnerabilities
|
||
|
||
- **Single shared virtual mic + stateful Opus decoder across concurrent sessions (no isolation)** — *refuted (downgraded to info).* Concurrent sessions are co-tenancy of ONE desktop by design (`punktfunk1.rs:244-246`); a paired client already injects keystrokes/captures that desktop via the identically-shared input service, so sharing the mic grants no new authority. Decoder "corruption" is self-healing (reopen) audio-quality, not security. Document the limitation alongside the known gamescope multi-user gap.
|
||
- **Cover-art warmer follows HTTP redirects → blind SSRF** — *refuted under the in-scope threat model.* URLs are hardcoded `https://api.gog.com` / `https://displaycatalog.mp.microsoft.com` constants reached over verified TLS (ureq 2.x → rustls + webpki-roots); the id is only a path segment. No in-scope adversary (network client, on-path MITM with no host key, local user) can emit the 30x `Location` — that requires a genuine compromise/cert-hijack of those domains (supply-chain, out of scope). A local user can only poison the path segment of a request still sent to the real upstream over TLS. Defense-in-depth: still set `.redirects(0)`.
|
||
- **GameStream RSA signing direct attacker-control** — partial-leaning: the adversary observes a timing side-channel, not a value flowing to a sink; see S7.
|
||
|
||
### (d) Positives confirmed on these surfaces
|
||
|
||
- **Native control plane is fail-closed at the app layer:** `serve_session` (`punktfunk1.rs:544-573`) rejects unpaired/anonymous clients before `validate_dimensions`, compositor resolution, the `can_encode_444` GPU probe, encoder open, and vdisplay create.
|
||
- **Client→host wire decoders are uniformly bounds-checked, no reachable parse panic/OOB:** `Hello.decode` uses checked `.get()` for every trailing field (the one `u32at` is gated by `len>=20`); `RichInput` (`quic.rs:1271`), `InputEvent` (`input.rs:136`), and `decode_mic_datagram` all length-check before indexed reads; unknown datagram tags are a non-fatal drop.
|
||
- **No client field reaches HDR SEI:** the 0xCE/HDR, 0xCA rumble, 0xCD HidOut datagrams are host→client only; the SEI builders are fed only host-derived values.
|
||
- **Geometry → unsafe FFI is memory-safe:** W/H caps applied on Hello, Reconfigure, and ANNOUNCE; CPU upload paths re-derive `src_row` from the encoder's own width and bound-check `bytes.len() >= src_row*h` before `sws_scale`/copy; encoder fully rebuilds on size change (no stale-size OOB); CUDA pitch math driver-bounded; Drop SAFETY contracts hold (no UAF/double-free); pf_vdisplay/SudoVDA ioctls use `size_of`-sized buffers with no attacker-controlled length.
|
||
- **Driver-IPC ABI is clean:** `pf-driver-proto` pins all offsets/sizes via compile-time `offset_of!`/`size_of` asserts; gamepad output reports, XUSB rumble, IDD-push publish token, and the WGC AU pipe all bounds-check before indexed reads and never use an attacker byte as offset/length/index; the only residual is the already-reported world-writable section ACL.
|
||
- **Opus mic buffer math exact:** 5760×2 f32 = the 120 ms max stereo frame; the safe `opus` crate returns `BufferTooSmall` rather than overflowing; `(samples×2).min(pcm.len())`.
|
||
- **Gamepad accumulation clamped at every layer:** `idx < MAX_WIRE_PADS(16)`, `idx >= MAX_PADS(4)` rejects, finger/touchpad/stick/trigger clamps.
|
||
- **No production GameStream client→host Opus decode path:** the only `MSDecoder::new(..., client_mapping)` call sites are inside `#[cfg(test)]` (the prompt's G1 premise corrected) — that attack surface does not exist in shipped code.
|
||
- **CLI default-security posture sound:** `require_pairing` / `open` use exact-string scans (malformed/quoted args can't flip them); the mgmt token is mandatory on every bind including loopback (`mgmt.rs:86-92,471-507`); empty `--mgmt-token` rejected; dev subcommands expose no weaker-trust default listener.
|
||
- **Cover-art direct SSRF safe:** hardcoded hosts, id only in path, TLS verification on, body capped at ureq's 10 MB; catalog art URLs flow only to clients, never re-fetched by the host.
|
||
- **Concurrency/probe bounds:** `max_concurrent` via owned semaphore permit before `accept()`; probe duration/rate clamped (`MAX_PROBE_MS=5s`, `MAX_PROBE_KBPS=10Gbps`); `ClockProbe` answered 1:1 (no amplification).
|
||
|
||
---
|
||
|
||
## Appendix — coverage-gap critic (pass 1) and how pass 2 addressed it
|
||
|
||
# Coverage gaps & follow-up
|
||
|
||
I enumerated all 82 host source files and mapped them to the 13 audit surfaces. Below are files / data-paths / cross-cutting concerns that **no surface clearly owns**, ranked for a follow-up pass.
|
||
|
||
## Gaps in per-file coverage
|
||
|
||
### G1 — Client mic-uplink Opus decode → privileged virtual mic (MED)
|
||
Files: `src/audio.rs`, `src/audio/linux/mod.rs`, `src/audio/windows/wasapi_cap.rs`+`wasapi_mic.rs`, decode sinks at `punktfunk1.rs:1233-1266` and `gamestream/audio.rs:610-732`.
|
||
The `native-protocol` surface covers the *demux* (0xCB → `mic_tx`) and `gamestream-wire` covers RTP framing, but the **Opus decode itself and the PCM injection into a host-wide virtual microphone** is owned by no surface. This is an attacker-controlled byte stream (`opus::Decoder::decode_float` on raw network bytes, `punktfunk1.rs:1266`) decoded into a system-visible recording device. Worse on the GameStream path: `gamestream/audio.rs:637/724` builds an `opus::MSDecoder` from a **client-derived channel mapping/layout** (`layout.streams`, `layout.coupled`, `client_mapping`) — verify those are bounds-checked before reaching libopus, and that decode errors can't panic/DoS the host-lifetime mic thread. Native path is post-auth; the GameStream mic path rides weaker GameStream trust. No audio-decode surface existed.
|
||
|
||
### G2 — Shared host-lifetime mic/input services across concurrent sessions (MED)
|
||
`punktfunk1.rs:219-300` (`mic_service` / `mic_tx` shared, host-lifetime). With `--max-concurrent` sessions sharing **one** virtual mic and input service, a paired client's mic stream / input can bleed into a *different* concurrent session's desktop. This spans `audio` + `session-lifecycle` + `input-injection` and no single surface would catch the cross-session isolation question. Adversary: post-auth client #2 against session #1 (multi-user isolation, explicitly listed as "remaining piece" in CLAUDE.md for gamescope).
|
||
|
||
### G3 — `main.rs` CLI parsing & default-security posture (MED)
|
||
`src/main.rs` (734 LOC) is owned by no surface. It decides the crown-jewel default: `require_pairing: !args.iter().any(|a| a == "--allow-tofu")` (`main.rs:388`) — a substring/exact-match flag scan that gates whether unpaired clients are accepted. Also hosts the `spike` and `punktfunk1-host` dev subcommands shipped in the production binary, and the `--mgmt-bind` parse (`main.rs:516`, non-loopback requires a token — good, but verify the loopback check can't be bypassed by `0.0.0.0`/IPv6-mapped forms). A default-posture/flag-parsing regression here silently disables pairing. Cross-cutting; no surface re-derives it.
|
||
|
||
### G4 — Cover-art warmer outbound egress + parse (LOW-MED)
|
||
`library.rs:1004-1090` (`fetch_gog_art`, `fetch_xbox_art`, host-lifetime warmer over `ureq`) and Epic `catcache.bin` base64 decode. `library-launch` likely covered launch-command construction, but the **outbound HTTP egress** (host as SSRF client fetching URLs influenced by on-disk store files / operator custom entries, `library.rs:481-697`) and the base64/JSON parse of attacker-influenceable launcher caches are a distinct trust boundary. Confirm `library-launch` actually traced the fetch side, not just launch exec.
|
||
|
||
### G5 — `hdr.rs` metadata path (LOW)
|
||
`src/hdr.rs` (168 LOC) — HDR/color-info construction. If any field derives from client `ColorInfo` (0xCE / connect_ex5 caps), it's attacker-influenced metadata fed to the encoder SEI. No surface names it.
|
||
|
||
### G6 — Glue/init files unmapped (LOW)
|
||
`pipeline.rs`, `pwinit.rs`, `session_tuning.rs`, `linux/dmabuf_fence.rs`, `linux/drm_sync.rs` — mostly internal glue, but the dmabuf/drm-sync FFI files border `unsafe-ffi`; confirm that surface's scope included them (they were not in its cited list of zerocopy/encode/capture).
|
||
|
||
## Cross-cutting concerns no per-surface review can catch
|
||
|
||
### X1 — Dependency / RUSTSEC posture (MED)
|
||
`Cargo.toml` is owned by no surface. Notable: **`rsa = "0.9"`** is subject to RUSTSEC-2023-0071 (Marvin timing side-channel) and is used directly by the **GameStream RSA pairing** ceremony — a network-adjacent oracle concern for `gamestream/crypto.rs`+`pairing.rs`. `ureq = "2"` backs the cover-art egress (G4). Run `cargo audit` against the workspace lock as a follow-up; per-surface reviewers won't.
|
||
|
||
### X2 — Secret-file create→chmod TOCTOU across modules (LOW)
|
||
`secrets-perms` verifies final perms, but the create-then-restrict ordering window is implemented independently in `gamestream/cert.rs`, `mgmt_token.rs`, `native_pairing.rs`, and the captures/art writers (`stats_recorder.rs`, `library.rs`). A single helper vs N call-sites is a cross-module check: confirm every secret is created with restrictive perms atomically (O_CREAT mode), not world-readable-then-chmod, on **every** path including ones added since the prior audit.
|
||
|
||
### X3 — On-disk capture / cache write paths (LOW)
|
||
`stats_recorder.rs` captures (`~/.config/punktfunk/captures/*.json`) and `library.rs` art cache are operator-readable artifacts; `stats-capture` covered the endpoints but confirm the **filename derivation** for saved captures can't be influenced by a network field (path traversal into the captures dir).
|
||
|
||
### X4 — `windows/install.rs` driver/web install moved into host exe (MED — verify owned)
|
||
`windows/install.rs` + `windows/interactive.rs` should be under `windows-service-priv`, but given commit 125a51d is new, explicitly confirm that surface traced: the source of bundled driver paths (pnputil install), any download/verify of the web bundle, and that `CreateProcessAsUserW`/scheduled-task launch can't be redirected by an unprivileged local user (adversary #4) writing into a host-readable staging dir.
|
||
|
||
Net: G1 (mic decode → virtual mic) and G3 (main.rs default posture) are the most likely real-blind-spots; X1 (rsa 0.9 in GameStream pairing) is the cleanest cross-cutting follow-up.
|