security: fix 13 findings from the 2026-08-15 whole-project sweep #244

Merged

enricobuehler merged 11 commits from worktree-security-review-2026-08-15-fixes into main 2026-08-15 09:37:10 +00:00

Conversation 0 Commits 11 Files Changed 20

+487 -40

enricobuehler commented 2026-08-15 09:36:41 +00:00

Owner

Copy Link

Copy Source

Works through the 2026-08-15 security review (punktfunk-planning/design/security-review-2026-08-15.md). Each finding was re-verified against current main first — the report's base 9c133350 was 119 commits stale, and one finding (F2's control-stream Connect owner-check) was already fixed on main.

Fixed (13 of 24)

#	Area	Fix
1	gamestream	media UDP recv-loops bind to the launch owner's IP (stream.rs/audio.rs)
2	gamestream	Event::Receive peer-gate + owner-filtering ENet socket
13	gamestream	OwnerFilteredSocket drops non-owner datagrams before ENet allocates peer state
11	core	reassembler now meters per-block BlockState against the in-flight budget (client-OOM)
9	pyrowave	push_packet min-size guard so a zero-length duplicate block can't spin the decode thread (vendored patch 0008)
6	web BFF	new provider/[provider].put.ts runs the console-password gate that the catch-all bypassed
5	CI	fork-PR cargo cache namespaced cargo-home-ci- so it can't poison the signed-release pool
14	CI	docs-deploy SSH step docker logouts the write:package PAT on exit
3	host/windows	load_host_env allow-list + is_admin_owned distrusts a non-admin-owned host.env before the re-own
4	host/windows	a planted (non-admin-owned) web-password is rotated instead of adopted
8	apple	library entry points gated on pinnedSHA256 != nil (no more silent TOFU)
7	gamestream	PinGate::submit refuses an ambiguous PIN when >1 handshake is parked (→409)
12 (win)	CI	bun windows zip pinned + sha256-verified before it enters the signed installer

Plus two low/informational hardenings: an HTTPResponse Content-Length overflow trap (Swift), and the art-path UNC guard now catches forward-slash/mixed UNC forms.

Build-verified

• Windows host on the windows-amd64 box (.133): cargo check -p punktfunk-host green — covers all #[cfg(windows)] code, audio.rs run(), and the pyrowave C++.
• Apple client via Xcode 26.6: swift build of PunktfunkClient green.
• punktfunk-core + cargo test reassemble green; cargo fmt --check clean.

Notes for review

• F5: I namespaced the cargo cache rather than add a fork guard that would disable fork-PR CI. The definitive control is Gitea's fork-PR approval gate.
• F7: converts the PIN-race into a pairing DoS-under-flood; the full fix keys the gate by uniqueid (mgmt API + console change).
• The GameStream findings (1/2/7/13) are on the runtime opt-in Moonlight-compat plane.

Deliberately deferred

• F10 (plugin launch = command execution): the real fix is a plugin wire-contract change (structured exe+argv) that breaks every existing plugin — needs plugin-side coordination.
• F12-linux: curl | bun.sh/install | bash sites (arch/rpm/deb + 3 builder Dockerfiles) still need version+hash pinning.
• F4's .iss FreshWebInstall signal (installer-side; the Rust side already neutralizes the planted password).
• Remaining low/info items.

Works through the 2026-08-15 security review (`punktfunk-planning/design/security-review-2026-08-15.md`). Each finding was re-verified against current `main` first — the report's base `9c133350` was 119 commits stale, and one finding (F2's control-stream Connect owner-check) was already fixed on main. ## Fixed (13 of 24) | # | Area | Fix | |---|------|-----| | 1 | gamestream | media UDP recv-loops bind to the launch owner's IP (`stream.rs`/`audio.rs`) | | 2 | gamestream | `Event::Receive` peer-gate + owner-filtering ENet socket | | 13 | gamestream | `OwnerFilteredSocket` drops non-owner datagrams before ENet allocates peer state | | 11 | core | reassembler now meters per-block `BlockState` against the in-flight budget (client-OOM) | | 9 | pyrowave | `push_packet` min-size guard so a zero-length duplicate block can't spin the decode thread (vendored patch `0008`) | | 6 | web BFF | new `provider/[provider].put.ts` runs the console-password gate that the catch-all bypassed | | 5 | CI | fork-PR cargo cache namespaced `cargo-home-ci-` so it can't poison the signed-release pool | | 14 | CI | docs-deploy SSH step `docker logout`s the `write:package` PAT on exit | | 3 | host/windows | `load_host_env` allow-list + `is_admin_owned` distrusts a non-admin-owned `host.env` before the re-own | | 4 | host/windows | a planted (non-admin-owned) `web-password` is rotated instead of adopted | | 8 | apple | library entry points gated on `pinnedSHA256 != nil` (no more silent TOFU) | | 7 | gamestream | `PinGate::submit` refuses an ambiguous PIN when >1 handshake is parked (→409) | | 12 (win) | CI | bun windows zip pinned + sha256-verified before it enters the signed installer | Plus two low/informational hardenings: an `HTTPResponse` `Content-Length` overflow trap (Swift), and the art-path UNC guard now catches forward-slash/mixed UNC forms. ## Build-verified - **Windows host** on the windows-amd64 box (.133): `cargo check -p punktfunk-host` green — covers all `#[cfg(windows)]` code, `audio.rs run()`, and the pyrowave C++. - **Apple client** via Xcode 26.6: `swift build` of `PunktfunkClient` green. - `punktfunk-core` + `cargo test reassemble` green; `cargo fmt --check` clean. ## Notes for review - **F5**: I namespaced the cargo cache rather than add a fork guard that would disable fork-PR CI. The definitive control is Gitea's fork-PR approval gate. - **F7**: converts the PIN-race into a pairing DoS-under-flood; the full fix keys the gate by `uniqueid` (mgmt API + console change). - The GameStream findings (1/2/7/13) are on the runtime opt-in Moonlight-compat plane. ## Deliberately deferred - **F10** (plugin launch = command execution): the real fix is a plugin **wire-contract** change (structured `exe`+`argv`) that breaks every existing plugin — needs plugin-side coordination. - **F12-linux**: `curl | bun.sh/install | bash` sites (arch/rpm/deb + 3 builder Dockerfiles) still need version+hash pinning. - **F4's `.iss`** `FreshWebInstall` signal (installer-side; the Rust side already neutralizes the planted password). - Remaining low/info items.

enricobuehler added 11 commits 2026-08-15 09:36:42 +00:00

fix(gamestream): bind the UDP/ENet media plane to the launch owner ... 6b3e793b39

security-review 2026-08-15 findings 1, 2, 13. The Moonlight-compat plane bound
its UDP video/audio endpoints to the first datagram from anyone and let any ENet
peer keep a connection (pinning per-peer reassembly memory) — the peer_ip the
RTSP/launch planes already enforce was never threaded to the media/control
sockets.

- stream.rs/audio.rs: the video/audio endpoint learn now discards datagrams whose
  source IP is not the launch owner's until the 10s budget is spent, so an
  off-path LAN peer can no longer win the endpoint race and be handed the
  (plaintext) video stream.
- control.rs: an OwnerFilteredSocket drops non-owner datagrams before ENet
  allocates any per-peer state (closes the ~32 MiB x peer_limit pin and the
  source-spoof injection variant), and the Event::Receive arm now honors only the
  tracked session peer's input as defense-in-depth.

GameStream is runtime opt-in and off in the shipped unit, so this is deferrable
but the code's own comments claimed a peer bind already protected these paths.

fix(core): meter per-block reassembly state against the in-flight budget ... 59d8b8a677

security-review 2026-08-15 finding 11. The reassembler's memory firewall counted
only FrameBuf::buf bytes; BlockState (have_data + recovery vectors, both sized
from attacker-declared header fields) was allocated unmetered. A slice-streamed
frame can mint thousands of distinct-index blocks while keeping the metered
buffer pinned near zero, committing multiple GB against a ~13 MB accounted
figure — a deterministic remote client OOM from a hostile/compromised host.

Add block_state_bytes()/frame_cost(); gate each new block on the same
IN_FLIGHT_BUF_FACTOR x max_frame_bytes budget as the frame buffer, and release
the full frame cost (buffer + block state) at every removal site.

fix(pyrowave): reject a sub-header packet so the decode cursor always advances ... 6ada74066d

security-review 2026-08-15 finding 9. push_packet advanced by
payload_words*4; a 12-bit payload_words of 0 passed the length guard, and
decode_packet's duplicate-block early return fired before its own minimum-size
check — so a duplicate block_index with payload_words==0 spun the client decode
thread at 100% CPU forever (no allocation, no timeout, inside FFI). Hoist the
minimum-size check into push_packet before decode_packet is consulted. Carried
as vendored patch 0008.

fix(web): gate PUT /api/v1/library/provider/{p} behind the console password ... ff33eb872e

security-review 2026-08-15 finding 6. confirmIfCommandExecution was wired into
only the two custom-entry routes; the provider reconcile route had no BFF handler
and fell through to the /api/** catch-all, which injects the full admin bearer —
so a bare session cookie could plant a persistent prep/launch.kind:command entry
without the password. Add the missing handler so it runs the same
command-execution gate before forwarding (an ordinary catalog reconcile is
untouched).

fix(ci): namespace the fork-PR cargo cache, log out the docs-deploy PAT ... 4369e3b2ec

security-review 2026-08-15 findings 5 and 14.

- ci.yml: the cargo-home cache shared its unnamespaced key with the signed
  release builds (deb.yml / android.yml). registry/src holds already-extracted
  crate sources cargo compiles without re-checksumming, so a fork PR could poison
  a release artifact through the shared pool. Namespace ci.yml's key to
  cargo-home-ci- so its (untrusted) caches never reach the release pool. The
  fork-approval gate remains the definitive operator-side control.
- docker.yml: the deploy-docs SSH step left a write:package PAT base64-encoded in
  ~/.docker/config.json on the long-lived internet-facing docs VM. Add a
  trap ... EXIT docker logout so it is cleared on every exit path, matching the
  ephemeral LAN-registry jobs.

fix(host/windows): allow-list host.env keys loaded into the SYSTEM service ... 6cd25b4829

security-review 2026-08-15 finding 3 (part 1). load_host_env imported EVERY key
of %ProgramData%\punktfunk\host.env into the LocalSystem service's own
environment. Since %ProgramData% lets BUILTIN\Users pre-create the dir, an
unprivileged user could plant host.env before install; a planted SystemRoot then
redirected the absolute icacls.exe / powershell.exe paths pf-paths and the
network-profile warner build from it — code execution as SYSTEM. Import only the
PUNKTFUNK_* / RUST_LOG keys the child already allow-lists at the spawn boundary,
closing the SystemRoot/PATH class of sinks.

Residual (planted PUNKTFUNK_HOST_CMD / PUNKTFUNK_CONFIG_DIR, which are legitimate
installer knobs) needs distrusting a non-admin-owned host.env — findings 3c/4,
which share an installer provisioning-signal decision and Windows build
verification; tracked, not yet fixed here.

fix(apple): require a pinned host identity before browsing its library ... 9a05750376

security-review 2026-08-15 finding 8. MgmtTransport's verify block replaces
system trust wholesale (the host cert is self-signed, no SAN) and, for a host
with no pinnedSHA256, accepted ANY certificate trust-on-first-use with no prompt
or log. A host can be saved yet pin-less (manual add, deep link, abandoned
pairing, or after Forget Identity), so a LAN MITM could serve a forged catalog
and harvest the device's mTLS pairing identity.

Gate the library entry points on host.pinnedSHA256 != nil — HomeView's browse
action, GamepadHomeView's hasLibrary tile flag, and a load() guard in LibraryView
(covering the deep-link path) — mirroring how the stream path already refuses an
unpinned connect. The transport's silent-accept is left for a follow-up (it is
also reached pre-pairing, so tightening it needs the QUIC path's approval flow).

NOTE: not compiled locally (no Xcode on the build host); verify on the Apple CI.

fix(host/windows): distrust a non-admin-owned host.env / web-password ... 662df795b2

security-review 2026-08-15 findings 3c and 4. %ProgramData% lets BUILTIN\Users
pre-create the punktfunk dir and plant host.env / web-password before a
privileged install runs; the bytes were then adopted verbatim (SYSTEM service
environment + command line; the console password), with the plant's owner erased
by the dir re-own that runs first.

Add install::is_admin_owned() (reads the file owner SID via GetNamedSecurityInfoW,
reusing privileged_sids()) and consult it BEFORE create_private_dir re-owns the
file:
- ensure_default_host_env: a non-admin-owned host.env is renamed aside and the
  default written over it (the !planted skip forces the overwrite even if the
  rename fails).
- set_web_password: a non-admin-owned password file is rotated to a fresh random
  instead of kept as an 'upgrade'.

A file from a prior privileged install is Administrators-owned and is kept.
Compiles clean on the windows-amd64 box (.133). The installer-side .iss freshness
signal (which also gates the password page) is a separate follow-up.

fix(gamestream): refuse an ambiguous pairing-PIN submit ... bb781ca694

security-review 2026-08-15 finding 7. The PIN is a single global slot with no
binding to a specific handshake, so with N parked getservercert waiters whichever
polls first takes it — an attacker who floods the parking slots while the
operator pairs could take the operator's PIN and pin its own certificate. Real
pairing is one client at a time, so PinGate::submit now refuses (returns false)
when more than one handshake is parked, and POST /pair/pin answers 409. This
narrows the window to a tight post-submit timing race; the full fix keys the gate
by uniqueid (mgmt API + console change, tracked separately). Compiles on .133.

fix(ci): verify the pinned bun windows zip by sha256 before staging it ... da13d14159

security-review 2026-08-15 finding 12 (windows). bun-windows-x64.zip was
downloaded and Expand-Archived with no integrity check, then Authenticode-signed
into the installer and its hash published in the Ed25519 update manifest — our
signature vouching for bytes we never verified (GitHub release assets are mutable
at a fixed URL). Pin and verify the sha256. The Linux curl|bash sites
(arch/rpm/deb + builder Dockerfiles) still need version+hash pinning — tracked.

harden: HTTP Content-Length overflow trap, and forward-slash UNC art paths ... 4aeec6051d

security-review 2026-08-15, two low/informational findings.

- clients/apple HTTPResponse: a malicious host sending Content-Length = Int.max
  made bodyStart + length overflow, and Swift integer overflow TRAPS (an
  uncatchable crash) rather than throwing. Use addingReportingOverflow and reject.
  Verified in the Apple build.
- host library/art.rs: art_path_is_confined's UNC guard was a leading double-
  backslash string test, so forward-slash (//server/share) and mixed UNC forms
  slipped past it, and canonicalize() itself would then coerce the SYSTEM host
  into outbound SMB auth. Reject ANY two leading path separators before touching
  the filesystem.

enricobuehler merged commit b8ec8ea260 into main 2026-08-15 09:37:10 +00:00

enricobuehler referenced this pull request 2026-08-15 09:51:37 +00:00

Per-client access control: grants + temporary guest access, host-enforced end to end (WP1–WP13) #245

enricobuehler referenced this issue from a commit 2026-08-17 12:48:39 +00:00

release: 0.30.0 — version bump, notes, CHANGELOG, Play notes

enricobuehler referenced this pull request 2026-08-17 12:49:03 +00:00

0.30.0 #285

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX FlipperDan HitFrostbite Seenko bluelightspecial enricobuehler (Enrico Bühler) kszlim luxus nannku neoney sonar

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#244