diff --git a/crates/punktfunk-host/src/encode.rs b/crates/punktfunk-host/src/encode.rs index 34781d0..238036c 100644 --- a/crates/punktfunk-host/src/encode.rs +++ b/crates/punktfunk-host/src/encode.rs @@ -230,6 +230,14 @@ pub fn open_video( chroma: ChromaFormat, ) -> Result> { validate_dimensions(codec, width, height)?; + // Refresh/fps must be positive and sane: fps feeds the encoder time_base (`Rational(1, fps)`) + // and the pts→ns conversion (`pts * 1e9 / fps`), so 0 builds a 1/0 rational / divides by zero. + // The mid-stream Reconfigure path already guards `refresh_hz > 0`; enforcing it at this single + // open chokepoint makes EVERY path (initial Hello, GameStream ANNOUNCE, Reconfigure) safe + // regardless of which backend opens (security-review 2026-06-28 S5). + if fps == 0 || fps > 1000 { + anyhow::bail!("invalid refresh/fps {fps}: must be 1..=1000 Hz"); + } // 4:4:4 is HEVC-only. The negotiator should never pass `Yuv444` for another codec (it gates on // `codec == H265`), but defend the contract here so a future caller can't silently emit a stream // no decoder expects: a non-HEVC 4:4:4 request degrades to 4:2:0 with a warning. diff --git a/crates/punktfunk-host/src/gamestream/control.rs b/crates/punktfunk-host/src/gamestream/control.rs index 23b3962..a420888 100644 --- a/crates/punktfunk-host/src/gamestream/control.rs +++ b/crates/punktfunk-host/src/gamestream/control.rs @@ -56,6 +56,9 @@ pub fn spawn(state: Arc) -> Result<()> { .spawn(move || { // GCM scheme detected from the first authenticating packet; reused thereafter. let mut detected: Option = None; + // Consecutive control-decrypt failures for this peer — throttles the warn log so a + // junk-packet flood can't spam unbounded lines (security-review 2026-06-28 #10). + let mut decrypt_fails: u64 = 0; // Decoded keyboard/mouse is forwarded to a dedicated host-lifetime injector thread — // NEVER injected inline, so a slow Wayland/libei/SendInput call can't head-block ENet // keepalive/retransmit servicing on this thread. The injector owns non-Send compositor @@ -77,6 +80,7 @@ pub fn spawn(state: Arc) -> Result<()> { Event::Disconnect { .. } => { tracing::info!("control: client disconnected"); detected = None; + decrypt_fails = 0; peer = None; // Unplug the session's virtual pads. pads = GamepadManager::new(); @@ -89,6 +93,7 @@ pub fn spawn(state: Arc) -> Result<()> { channel_id, packet.data(), &mut detected, + &mut decrypt_fails, &inj_tx, &mut pads, ); @@ -163,6 +168,7 @@ fn on_receive( _channel_id: u8, d: &[u8], detected: &mut Option, + decrypt_fails: &mut u64, inj_tx: &Sender, pads: &mut GamepadManager, ) { @@ -180,10 +186,20 @@ fn on_receive( tracing::info!(?scheme, "control: GCM scheme locked in"); } *detected = Some(scheme); + *decrypt_fails = 0; pt } None => { - tracing::warn!(len = d.len(), "control: GCM decrypt failed"); + // Throttle: a junk-packet flood must not spam one warn line per packet. Log the first + // failure, then only at exponentially-spaced counts (1, 2, 4, 8, …). + *decrypt_fails += 1; + if decrypt_fails.is_power_of_two() { + tracing::warn!( + len = d.len(), + fails = *decrypt_fails, + "control: GCM decrypt failed" + ); + } return; } }; diff --git a/crates/punktfunk-host/src/gamestream/mod.rs b/crates/punktfunk-host/src/gamestream/mod.rs index b820df2..e304bb3 100644 --- a/crates/punktfunk-host/src/gamestream/mod.rs +++ b/crates/punktfunk-host/src/gamestream/mod.rs @@ -90,6 +90,11 @@ pub struct LaunchSession { pub fps: u32, /// `/launch?appid=N` — selects the app-catalog entry (session recipe). pub appid: u32, + /// Source IP of the paired HTTPS client that issued `/launch`. The unauthenticated RTSP/UDP + /// media plane binds to this so only the launching peer can start/own the stream — an + /// unpaired RTSP peer cannot ride a paired client's launch (security-review 2026-06-28 #4). + /// `None` if the address could not be captured (then RTSP falls back to launch-present only). + pub peer_ip: Option, } /// Shared control-plane state used as the axum app state. diff --git a/crates/punktfunk-host/src/gamestream/nvhttp.rs b/crates/punktfunk-host/src/gamestream/nvhttp.rs index 53df421..67e4961 100644 --- a/crates/punktfunk-host/src/gamestream/nvhttp.rs +++ b/crates/punktfunk-host/src/gamestream/nvhttp.rs @@ -1,9 +1,14 @@ //! The nvhttp servers: plain HTTP on 47989 and mutual-TLS on 47984. Serves `/serverinfo`, -//! the `/pair` flow, `/applist`, and `/launch`/`/resume`/`/cancel`, plus a punktfunk-only -//! `/pin` endpoint to deliver the Moonlight-displayed PIN. Over HTTPS the client is +//! the `/pair` flow, `/applist`, and `/launch`/`/resume`/`/cancel`. Over HTTPS the client is //! mutual-TLS-authenticated, so `/serverinfo` reports `PairStatus=1` there. +//! +//! The pairing PIN is delivered out-of-band ONLY through the bearer-authenticated management +//! API (`POST /api/v1/pair/pin`): the operator reads the PIN off the Moonlight client and +//! types it into the host console. There is deliberately NO unauthenticated nvhttp PIN +//! endpoint — one would let a network client submit its own displayed PIN and drive the whole +//! ceremony to a pinned cert with no operator consent (security-review 2026-06-28 #1). -use super::tls::PeerCertFingerprint; +use super::tls::{PeerAddr, PeerCertFingerprint}; use super::{serverinfo, AppState, LaunchSession, HTTPS_PORT, HTTP_PORT, RTSP_PORT}; use anyhow::{anyhow, Context, Result}; use axum::{ @@ -58,7 +63,6 @@ fn router(state: Arc, https: bool) -> Router { Router::new() .route("/serverinfo", get(h_serverinfo)) .route("/pair", get(h_pair)) - .route("/pin", get(h_pin)) .route("/applist", get(h_applist)) .route("/launch", get(h_launch)) .route("/resume", get(h_resume)) @@ -82,19 +86,6 @@ async fn h_serverinfo( xml(serverinfo::serverinfo_xml(&st.host, https, paired)) } -async fn h_pin( - State(st): State>, - Query(q): Query>, -) -> impl IntoResponse { - match q.get("pin").filter(|p| !p.is_empty()) { - Some(pin) => { - st.pairing.pin.submit(pin.clone()); - "PIN accepted\n".to_string() - } - None => "usage: GET /pin?pin=NNNN\n".to_string(), - } -} - async fn h_applist( State(st): State>, peer: Option>, @@ -110,6 +101,7 @@ async fn h_applist( async fn h_launch( State(st): State>, peer: Option>, + addr: Option>, Query(q): Query>, ) -> impl IntoResponse { if !peer_is_paired(&peer, &st) { @@ -117,7 +109,9 @@ async fn h_launch( return xml(error_xml()); } match launch(&st, &q) { - Ok(session) => { + Ok(mut session) => { + // Bind the (unauthenticated) RTSP/UDP media plane to this paired client's source IP. + session.peer_ip = addr.map(|Extension(PeerAddr(a))| a.ip()); *st.launch.lock().unwrap() = Some(session); tracing::info!( w = session.width, @@ -193,6 +187,7 @@ fn launch(_st: &AppState, q: &HashMap) -> Result height, fps, appid, + peer_ip: None, // set by `h_launch` from the verified HTTPS peer address }) } diff --git a/crates/punktfunk-host/src/gamestream/pairing.rs b/crates/punktfunk-host/src/gamestream/pairing.rs index f763a01..0f85f37 100644 --- a/crates/punktfunk-host/src/gamestream/pairing.rs +++ b/crates/punktfunk-host/src/gamestream/pairing.rs @@ -17,9 +17,14 @@ use std::sync::Mutex; use std::time::Duration; use tokio::sync::Notify; -/// Out-of-band PIN delivery. Moonlight generates + displays a PIN; the user submits it -/// (via the management API's `POST /api/v1/pair/pin` or nvhttp's `GET /pin?pin=NNNN`). -/// `getservercert` parks until a PIN arrives. +/// Out-of-band PIN delivery. Moonlight generates + displays a PIN; the operator submits it +/// via the bearer-authenticated management API (`POST /api/v1/pair/pin`) only — there is no +/// unauthenticated nvhttp delivery path (a network client must never be able to submit its +/// own PIN; security-review 2026-06-28 #1). `getservercert` parks until a PIN arrives. +/// Max pairing handshakes parked in [`PinGate::take`] at once (each holds a slot for up to +/// 300s), bounding a pre-auth waiter flood. Real pairing is one operator-driven client at a time. +const MAX_PARKED_WAITERS: usize = 4; + pub struct PinGate { pin: Mutex>, notify: Notify, @@ -48,7 +53,20 @@ impl PinGate { } async fn take(&self, timeout: Duration) -> Option { - self.waiters.fetch_add(1, Ordering::SeqCst); + // Bound the number of pairing handshakes parked at once: each `getservercert` is + // pre-auth and parks for up to 300s, so without a cap an unpaired LAN peer could pin + // unbounded tasks + keep `awaiting_pin` asserted (security-review 2026-06-28 #12). + // Reserve a slot atomically; refuse (treated as "no PIN") once the cap is reached. + if self + .waiters + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |n| { + (n < MAX_PARKED_WAITERS).then_some(n + 1) + }) + .is_err() + { + tracing::warn!("pairing: too many handshakes awaiting a PIN — refusing"); + return None; + } // Decrement on every exit path (PIN delivered, timeout, or future cancellation). struct WaiterGuard<'a>(&'a AtomicUsize); impl Drop for WaiterGuard<'_> { @@ -117,7 +135,8 @@ impl Pairing { tracing::info!( uniqueid, - "pairing phase 1 (getservercert) — awaiting PIN: submit `GET /pin?pin=NNNN`" + "pairing phase 1 (getservercert) — awaiting PIN: deliver it via the management \ + API `POST /api/v1/pair/pin` (operator reads the PIN off the Moonlight client)" ); let pin = self .pin @@ -304,4 +323,28 @@ mod tests { assert_eq!(pairing.pin.take(Duration::from_millis(10)).await, None); assert!(!pairing.pin.awaiting_pin()); } + + /// A pre-auth peer flood can park at most `MAX_PARKED_WAITERS` pairing handshakes; the next + /// `take` is refused immediately (returns `None` without parking), bounding the 300s-waiter DoS + /// (security-review 2026-06-28 #12). + #[tokio::test] + async fn pin_gate_caps_parked_waiters() { + let pairing = Arc::new(Pairing::new()); + let mut handles = Vec::new(); + for _ in 0..MAX_PARKED_WAITERS { + let p = pairing.clone(); + handles.push(tokio::spawn(async move { + p.pin.take(Duration::from_secs(5)).await + })); + } + // Wait until all the slots are taken. + while pairing.pin.waiters.load(Ordering::SeqCst) < MAX_PARKED_WAITERS { + tokio::time::sleep(Duration::from_millis(2)).await; + } + // One more is refused right away (no parking), even with a long timeout. + assert_eq!(pairing.pin.take(Duration::from_secs(5)).await, None); + for h in handles { + h.abort(); + } + } } diff --git a/crates/punktfunk-host/src/gamestream/rtsp.rs b/crates/punktfunk-host/src/gamestream/rtsp.rs index f2087b6..9da60b5 100644 --- a/crates/punktfunk-host/src/gamestream/rtsp.rs +++ b/crates/punktfunk-host/src/gamestream/rtsp.rs @@ -14,7 +14,7 @@ use crate::encode::Codec; use anyhow::{Context, Result}; use std::collections::HashMap; use std::io::{Read, Write}; -use std::net::{TcpListener, TcpStream}; +use std::net::{SocketAddr, TcpListener, TcpStream}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::time::Duration; @@ -102,13 +102,12 @@ fn handle_conn(mut stream: TcpStream, state: Arc) -> Result<()> { "RTSP {} | {}", req.head.replace("\r\n", " | "), if req.body.is_empty() { String::new() } else { format!("body: {}", req.body.replace("\r\n", " | ")) } ); - let resp = handle_request(&req, &state); + let resp = handle_request(&req, &state, peer); stream.write_all(resp.as_bytes()).context("RTSP write")?; stream.flush().ok(); // Close (FIN after the flushed response) so the client detects end-of-response. let _ = stream.shutdown(std::net::Shutdown::Both); } - let _ = peer; Ok(()) } @@ -171,7 +170,7 @@ fn parse_request(head: &str, body: String) -> Request { } } -fn handle_request(req: &Request, state: &AppState) -> String { +fn handle_request(req: &Request, state: &AppState, peer: Option) -> String { match req.method.as_str() { "OPTIONS" => response( &req.cseq, @@ -216,16 +215,30 @@ fn handle_request(req: &Request, state: &AppState) -> String { response(&req.cseq, &[], None) } "PLAY" => { + // The RTSP/UDP media plane is UNAUTHENTICATED. A stream may start only for the paired + // client that completed the pairing-gated `/launch` (which set `state.launch`), and — + // when the launching IP is known — only from that same source IP. So an unpaired RTSP + // peer can neither start a stream on an idle host nor ride a paired client's active + // launch (security-review 2026-06-28 #4). `nvhttp` gates `/launch` on a pinned cert. + let launch = *state.launch.lock().unwrap(); + let Some(ls) = launch else { + tracing::warn!(?peer, "RTSP PLAY — refused: no paired `/launch` session"); + return response_status("401 Unauthorized", &req.cseq, &[], None); + }; + if let (Some(want), Some(got)) = (ls.peer_ip, peer.map(|p| p.ip())) { + if want != got { + tracing::warn!( + %want, %got, + "RTSP PLAY — refused: peer IP does not match the launching client" + ); + return response_status("401 Unauthorized", &req.cseq, &[], None); + } + } let cfg = *state.stream.lock().unwrap(); match cfg { Some(cfg) if !state.streaming.swap(true, Ordering::SeqCst) => { // Resolve the launched catalog entry (session recipe) for the stream. - let app = state - .launch - .lock() - .unwrap() - .map(|l| l.appid) - .and_then(super::apps::by_id); + let app = super::apps::by_id(ls.appid); tracing::info!(app = ?app.as_ref().map(|a| &a.title), "RTSP PLAY — starting video stream"); stream::start( cfg, @@ -243,18 +256,15 @@ fn handle_request(req: &Request, state: &AppState) -> String { // Audio runs independently (Opus on UDP 48000, stereo or 5.1/7.1 multistream per // the ANNOUNCE); it needs the launch key for the AES-CBC payload encryption the // client expects. - let launch = *state.launch.lock().unwrap(); - if let Some(ls) = launch { - if !state.audio_streaming.swap(true, Ordering::SeqCst) { - tracing::info!("RTSP PLAY — starting audio stream"); - audio::start( - state.audio_streaming.clone(), - ls.gcm_key, - ls.rikeyid, - *state.audio_params.lock().unwrap(), - state.audio_cap.clone(), - ); - } + if !state.audio_streaming.swap(true, Ordering::SeqCst) { + tracing::info!("RTSP PLAY — starting audio stream"); + audio::start( + state.audio_streaming.clone(), + ls.gcm_key, + ls.rikeyid, + *state.audio_params.lock().unwrap(), + state.audio_cap.clone(), + ); } response(&req.cseq, &[("Session", "DEADBEEFCAFE;timeout = 90")], None) } diff --git a/crates/punktfunk-host/src/gamestream/tls.rs b/crates/punktfunk-host/src/gamestream/tls.rs index 8ddf2dc..45fd2bb 100644 --- a/crates/punktfunk-host/src/gamestream/tls.rs +++ b/crates/punktfunk-host/src/gamestream/tls.rs @@ -24,6 +24,12 @@ use std::sync::Arc; #[derive(Clone)] pub(crate) struct PeerCertFingerprint(pub Option); +/// The TCP source address of an HTTPS request, injected per-connection by [`serve_https`]. Used by +/// `/launch` to record which paired client owns the session so the unauthenticated RTSP/UDP media +/// plane can bind to that peer's IP (security-review 2026-06-28 #4). +#[derive(Clone, Copy)] +pub(crate) struct PeerAddr(pub SocketAddr); + /// HTTPS server that surfaces the verified client cert to handlers. `axum_server` can't expose the /// peer cert, so this runs the rustls handshake itself (tokio-rustls), reads the peer certificate, /// and serves the axum `Router` over hyper with the peer's fingerprint attached to every request as @@ -39,7 +45,7 @@ pub(crate) async fn serve_https( .await .with_context(|| format!("bind HTTPS {bind}"))?; loop { - let (tcp, _peer) = match listener.accept().await { + let (tcp, peer) = match listener.accept().await { Ok(v) => v, Err(e) => { tracing::warn!(error = %e, "HTTPS accept failed"); @@ -63,14 +69,16 @@ pub(crate) async fn serve_https( .peer_certificates() .and_then(|c| c.first()) .map(|c| hex::encode(punktfunk_core::quic::endpoint::cert_fingerprint(c.as_ref()))); - let peer = PeerCertFingerprint(fp); + let fp = PeerCertFingerprint(fp); + let addr = PeerAddr(peer); let svc = hyper::service::service_fn(move |req: hyper::Request| { let app = app.clone(); - let peer = peer.clone(); + let fp = fp.clone(); async move { let mut req = req.map(axum::body::Body::new); - req.extensions_mut().insert(peer); + req.extensions_mut().insert(fp); + req.extensions_mut().insert(addr); app.oneshot(req).await // Router error is Infallible } }); diff --git a/crates/punktfunk-host/src/inject.rs b/crates/punktfunk-host/src/inject.rs index 2fc164d..ae24440 100644 --- a/crates/punktfunk-host/src/inject.rs +++ b/crates/punktfunk-host/src/inject.rs @@ -76,9 +76,7 @@ pub fn open(backend: Backend) -> Result> { #[cfg(target_os = "linux")] { Ok(Box::new(libei::LibeiInjector::open_with( - libei::EiSource::SocketPathFile( - crate::vdisplay::gamescope_ei_socket_file().into(), - ), + libei::EiSource::SocketPathFile(crate::vdisplay::gamescope_ei_socket_file()), )?)) } #[cfg(not(target_os = "linux"))] diff --git a/crates/punktfunk-host/src/inject/linux/libei.rs b/crates/punktfunk-host/src/inject/linux/libei.rs index 1cebdf3..6978893 100644 --- a/crates/punktfunk-host/src/inject/linux/libei.rs +++ b/crates/punktfunk-host/src/inject/linux/libei.rs @@ -305,6 +305,19 @@ async fn connect_socket_file(file: &std::path::Path) -> Result { let deadline = std::time::Instant::now() + Duration::from_secs(15); let mut logged = String::new(); loop { + // Defense-in-depth: never follow a symlinked relay file. It lives under `$XDG_RUNTIME_DIR` + // (per-user 0700) so a cross-user plant is already blocked, but refuse a symlink outright + // rather than read through one to an attacker-chosen target (a rogue EIS server would + // keylog/deny the session's input; security-review 2026-06-28 #6). + if std::fs::symlink_metadata(file) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + return Err(anyhow!( + "EIS relay file {} is a symlink — refusing to follow it", + file.display() + )); + } if let Ok(s) = std::fs::read_to_string(file) { let name = s.trim(); if !name.is_empty() { diff --git a/crates/punktfunk-host/src/library.rs b/crates/punktfunk-host/src/library.rs index 674caf1..fd355b8 100644 --- a/crates/punktfunk-host/src/library.rs +++ b/crates/punktfunk-host/src/library.rs @@ -577,10 +577,11 @@ impl LibraryProvider for EpicProvider { if p.extension().and_then(|e| e.to_str()) != Some("item") { continue; } - let Ok(text) = std::fs::read_to_string(&p) else { + // `.item` manifests are small JSON; cap the read so a planted giant can't OOM the host. + let Some(bytes) = read_capped(&p, 1024 * 1024) else { continue; }; - let Ok(v) = serde_json::from_str::(&text) else { + let Ok(v) = serde_json::from_slice::(&bytes) else { continue; }; if let Some(g) = epic_entry(&v, &art) { @@ -650,6 +651,23 @@ fn epic_entry( }) } +/// Read a launcher cache/manifest with a hard size cap, so a local unprivileged user can't plant a +/// multi-GB file under the launcher's (Users-writable) data dir that OOMs the privileged host when +/// it's loaded — then base64/JSON-decoded into further copies — during library enumeration +/// (security-review 2026-06-28 S4). Returns `None` if missing, empty, or over `max`. Mirrors the +/// Linux lutris-art reader's 1 MiB cap. +#[cfg(windows)] +fn read_capped(path: &Path, max: u64) -> Option> { + let meta = std::fs::metadata(path).ok()?; + if meta.len() == 0 || meta.len() > max { + if meta.len() > max { + tracing::warn!(path = %path.display(), len = meta.len(), max, "launcher cache exceeds size cap — skipping"); + } + return None; + } + std::fs::read(path).ok() +} + /// Best-effort parse of `catcache.bin` (base64-encoded JSON array of catalog items) into /// catalogItemId → [`Artwork`] from each item's `keyImages`. Empty map on any read/decode failure /// (the format is community-reverse-engineered + can lag a fresh install → titles just show no art). @@ -657,7 +675,8 @@ fn epic_entry( fn epic_art_index(catcache: &Path) -> std::collections::HashMap { use base64::Engine as _; let mut map = std::collections::HashMap::new(); - let Ok(raw) = std::fs::read(catcache) else { + // 32 MiB cap: comfortably fits a real catalog cache, blocks a planted giant (S4). + let Some(raw) = read_capped(catcache, 32 * 1024 * 1024) else { return map; }; let Ok(decoded) = base64::engine::general_purpose::STANDARD.decode(raw) else { diff --git a/crates/punktfunk-host/src/mgmt.rs b/crates/punktfunk-host/src/mgmt.rs index b13e6a9..e3c2790 100644 --- a/crates/punktfunk-host/src/mgmt.rs +++ b/crates/punktfunk-host/src/mgmt.rs @@ -1680,6 +1680,7 @@ mod tests { height: 1440, fps: 120, appid: 1, + peer_ip: None, }); state.streaming.store(true, Ordering::SeqCst); @@ -1805,6 +1806,7 @@ mod tests { height: 1080, fps: 60, appid: 1, + peer_ip: None, }); let del = axum::http::Request::delete("/api/v1/session") diff --git a/crates/punktfunk-host/src/punktfunk1.rs b/crates/punktfunk-host/src/punktfunk1.rs index 00d1542..7de9d73 100644 --- a/crates/punktfunk-host/src/punktfunk1.rs +++ b/crates/punktfunk-host/src/punktfunk1.rs @@ -497,7 +497,7 @@ async fn serve_session( opts: &Punktfunk1Options, audio_cap: &AudioCapSlot, inj_tx: std::sync::mpsc::Sender, - mic_tx: std::sync::mpsc::Sender>, + mic_tx: std::sync::mpsc::SyncSender>, host_fp: &[u8; 32], np: &NativePairing, last_pairing: &std::sync::Mutex>, @@ -597,9 +597,11 @@ async fn serve_session( // we look it up in OUR library so a client can't inject a command). The bare-spawn gamescope // backend picks this up via the `PUNKTFUNK_GAMESCOPE_APP` env fallback in `spawn` (on a shared // desktop / attach-to-existing session it's a harmless no-op). This is the process-global env - // path — safe under today's ONE-session-at-a-time model; when concurrent native sessions land - // (`what's left` §3), resolve the command into the per-session VirtualDisplay via - // `set_launch_command` (as the GameStream path now does) so sessions can't stomp each other. + // path; the write is serialized via `vdisplay::with_env_lock` so concurrent native-session + // handshakes can't race the `set_var` (security-review 2026-06-28 #7). The remaining + // cross-session *value* confusion (B's launch id stomping A's pending gamescope spawn) wants + // the command resolved into the per-session VirtualDisplay via `set_launch_command` (as the + // GameStream path does) — a follow-up; the data-race UB is closed here. if let Some(id) = hello.launch.as_deref() { // Linux: resolve the id to a gamescope-nested command and stash it in the env the // gamescope backend reads. Windows has no gamescope to nest into — the data plane launches @@ -609,7 +611,9 @@ async fn serve_session( match crate::library::launch_command(id) { Some(cmd) => { tracing::info!(launch_id = id, command = %cmd, "launching library title"); - std::env::set_var("PUNKTFUNK_GAMESCOPE_APP", &cmd); + crate::vdisplay::with_env_lock(|| { + std::env::set_var("PUNKTFUNK_GAMESCOPE_APP", &cmd) + }); } None => tracing::warn!( launch_id = id, @@ -907,8 +911,9 @@ async fn serve_session( while let Ok(d) = input_conn.read_datagram().await { if let Some((_seq, _pts, opus)) = punktfunk_core::quic::decode_mic_datagram(&d) { mic_count += 1; - // Host-lifetime mic service; a send error just means the host is shutting down. - let _ = mic_tx.send(opus.to_vec()); + // Host-lifetime mic service (bounded queue): `try_send` drops the frame when the + // service is full or gone, never blocking this datagram loop (security-review S6). + let _ = mic_tx.try_send(opus.to_vec()); } else if let Some(rich) = punktfunk_core::quic::RichInput::decode(&d) { rich_count += 1; if rich_tx.send(rich).is_err() { @@ -1185,6 +1190,8 @@ const INJECTOR_REOPEN_BACKOFF: std::time::Duration = std::time::Duration::from_s /// Mic is 48 kHz stereo — matches the Opus stereo decoder and the host→client audio layout. const MIC_CHANNELS: u32 = 2; +/// Bound for the shared mic frame queue (drop-newest when full). See [`MicService::start`]. +const MIC_QUEUE_CAP: usize = 64; /// Host-lifetime virtual microphone, shared across punktfunk/1 sessions (mirror of /// [`InjectorService`]). One thread owns the PipeWire `Audio/Source` + an Opus decoder; sessions @@ -1192,12 +1199,16 @@ const MIC_CHANNELS: u32 = 2; /// feeds the source. Opened lazily on the first frame, the source node persists across sessions /// (no per-session registration churn), and reopens after a backoff if the source/decoder fails. struct MicService { - tx: std::sync::mpsc::Sender>, + tx: std::sync::mpsc::SyncSender>, } impl MicService { fn start() -> MicService { - let (tx, rx) = std::sync::mpsc::channel::>(); + // Bounded so the host-lifetime mic queue (shared across all concurrent sessions) can't grow + // without limit under a near-line-rate flood; the producer drops the newest frame when full + // (audio is lossy by design) rather than buffering unboundedly (security-review 2026-06-28 + // S6). 64 × 5–10 ms frames ≈ 0.3–0.6 s of slack, far more than the decode loop ever lags. + let (tx, rx) = std::sync::mpsc::sync_channel::>(MIC_QUEUE_CAP); if let Err(e) = std::thread::Builder::new() .name("punktfunk1-mic".into()) .spawn(move || mic_service_thread(rx)) @@ -1209,7 +1220,7 @@ impl MicService { /// A sender a session forwards the client's Opus mic frames to. Cloned per session; dropping a /// clone does NOT stop the service (it holds the original sender for the host life). - fn sender(&self) -> std::sync::mpsc::Sender> { + fn sender(&self) -> std::sync::mpsc::SyncSender> { self.tx.clone() } } @@ -1224,14 +1235,17 @@ fn mic_service_thread(rx: std::sync::mpsc::Receiver>) { /// The host-lifetime mic worker: lazily open the virtual mic + decoder, then Opus-decode each /// forwarded frame and push the PCM into the source. Reopen (after [`INJECTOR_REOPEN_BACKOFF`]) -/// on open failure or a decode error. Exits when every session sender and the service's own -/// sender drop (host shutdown), tearing the virtual mic down. Linux = PipeWire `Audio/Source`; -/// Windows = a virtual audio device's render endpoint (see `audio::wasapi_mic`). +/// only on a backend OPEN failure; a per-frame Opus DECODE error is just a dropped frame (it must +/// not tear down this mic, which is shared across every concurrent session — otherwise one paired +/// client's junk frames would deny everyone's mic; security-review 2026-06-28 S2). Exits when every +/// session sender and the service's own sender drop (host shutdown), tearing the virtual mic down. +/// Linux = PipeWire `Audio/Source`; Windows = a virtual audio device's render endpoint. #[cfg(any(target_os = "linux", target_os = "windows"))] fn mic_service_thread(rx: std::sync::mpsc::Receiver>) { let mut mic: Option> = None; let mut decoder: Option = None; let mut last_failed: Option = None; + let mut decode_fails: u64 = 0; let mut pcm = vec![0f32; 5760 * MIC_CHANNELS as usize]; // up to 120 ms scratch for opus_frame in rx { if opus_frame.is_empty() { @@ -1267,12 +1281,16 @@ fn mic_service_thread(rx: std::sync::mpsc::Receiver>) { Ok(samples_per_ch) => { let total = (samples_per_ch * MIC_CHANNELS as usize).min(pcm.len()); m.push(&pcm[..total]); + decode_fails = 0; } Err(e) => { - tracing::warn!(error = %e, "mic opus decode failed — reopening"); - mic = None; - decoder = None; - last_failed = Some(std::time::Instant::now()); + // Malformed/garbage frame: drop it and keep the (shared) mic + decoder open. The + // next valid frame decodes normally; only a backend OPEN failure reopens. Throttle + // the log (1, 2, 4, … fails) so a junk flood can't spam. + decode_fails += 1; + if decode_fails.is_power_of_two() { + tracing::warn!(error = %e, fails = decode_fails, "mic opus decode failed — dropping frame"); + } } } } @@ -1454,8 +1472,14 @@ fn input_thread( // left-button-down then turns every later click into a drag: windows move, but clicking buttons // and text inputs does nothing). We synthesize the matching up-events when this session ends — // see the release loop after the `break`. - let mut held_buttons: Vec = Vec::new(); - let mut held_keys: Vec = Vec::new(); + // Sets (not Vecs) so the presence test is O(1), not O(n) per event, and bounded by `MAX_HELD` + // so a client flooding distinct never-released codes can't grow the tracking state or spike the + // input thread (security-review 2026-06-28 S3). A real keyboard+mouse holds far fewer at once; + // codes past the cap simply aren't tracked for end-of-session release (worst case: one unreleased + // key on a pathological disconnect, which the injector's own state still bounds). + const MAX_HELD: usize = 256; + let mut held_buttons: std::collections::HashSet = std::collections::HashSet::new(); + let mut held_keys: std::collections::HashSet = std::collections::HashSet::new(); loop { match rx.recv_timeout(std::time::Duration::from_millis(4)) { Ok(ev) => match ev.kind { @@ -1473,14 +1497,18 @@ fn input_thread( _ => { // Track press/release so a mid-press disconnect can be undone below. match ev.kind { - InputKind::MouseButtonDown if !held_buttons.contains(&ev.code) => { - held_buttons.push(ev.code) + InputKind::MouseButtonDown if held_buttons.len() < MAX_HELD => { + held_buttons.insert(ev.code); } - InputKind::MouseButtonUp => held_buttons.retain(|&c| c != ev.code), - InputKind::KeyDown if !held_keys.contains(&ev.code) => { - held_keys.push(ev.code) + InputKind::MouseButtonUp => { + held_buttons.remove(&ev.code); + } + InputKind::KeyDown if held_keys.len() < MAX_HELD => { + held_keys.insert(ev.code); + } + InputKind::KeyUp => { + held_keys.remove(&ev.code); } - InputKind::KeyUp => held_keys.retain(|&c| c != ev.code), _ => {} } // Pointer/keyboard → the host-lifetime injector service (one persistent diff --git a/crates/punktfunk-host/src/vdisplay.rs b/crates/punktfunk-host/src/vdisplay.rs index 92e7fed..4da846c 100644 --- a/crates/punktfunk-host/src/vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay.rs @@ -358,13 +358,30 @@ fn find_wayland_socket(runtime: &str, uid: u32) -> Option { cands.into_iter().next().map(|(_, n)| n) } +/// Serializes ALL process-global env mutation on the per-session setup path. `std::env::set_var` +/// concurrent with another thread's `set_var` (glibc `environ` realloc) is a data race = UB. With +/// the default concurrent native sessions each running `resolve_compositor` in its own +/// `spawn_blocking`, the per-session env retargeting would otherwise race and could crash the host +/// (security-review 2026-06-28 #7). Every env write on the setup path takes this lock; steady-state +/// streaming reads cached config, not env. This removes the memory-unsafety; it is NOT a full fix +/// for cross-session env *value* confusion (that needs per-session `SessionContext` threading, as the +/// GameStream/Windows path already does via `set_launch_command`). +pub static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +/// Run `f` with [`ENV_LOCK`] held. Use around any `set_var`/`remove_var` on the session-setup path. +pub fn with_env_lock(f: impl FnOnce() -> R) -> R { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + f() +} + /// Write a detected session's [`SessionEnv`] into the process env so every backend (video capture /// and input alike) that reads `WAYLAND_DISPLAY` / `XDG_RUNTIME_DIR` / `DBUS_SESSION_BUS_ADDRESS` / -/// `XDG_CURRENT_DESKTOP` at open time targets the live session. The host serves one session at a -/// time, so a process-global write is sound; the next connect re-detects and re-applies. Same -/// `set_var` discipline already used for `PUNKTFUNK_GAMESCOPE_APP` on the launch path. +/// `XDG_CURRENT_DESKTOP` at open time targets the live session. Serialized via [`ENV_LOCK`] so +/// concurrent session handshakes can't race the `set_var`s; the next connect re-detects and +/// re-applies. Same `set_var` discipline used for `PUNKTFUNK_GAMESCOPE_APP` on the launch path. #[cfg(target_os = "linux")] pub fn apply_session_env(active: &ActiveSession) { + let _env_guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let e = &active.env; std::env::set_var("XDG_RUNTIME_DIR", &e.xdg_runtime_dir); std::env::set_var("DBUS_SESSION_BUS_ADDRESS", &e.dbus_session_bus_address); @@ -455,6 +472,7 @@ pub fn settle_desktop_portal(_chosen: Compositor) {} /// `PUNKTFUNK_GAMESCOPE_MANAGED` forces managed over either. #[cfg(target_os = "linux")] pub fn apply_input_env(chosen: Compositor) { + let _env_guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let backend = match chosen { Compositor::Gamescope => "gamescope", // KWin: org_kde_kwin_fake_input — direct injection, no RemoteDesktop portal / approval @@ -587,10 +605,10 @@ pub fn probe(compositor: Compositor) -> Result<()> { } /// Path of the file where the gamescope backend relays the nested session's `LIBEI_SOCKET` -/// (gamescope's EIS server) for the input injector. +/// (gamescope's EIS server) for the input injector. Under `$XDG_RUNTIME_DIR` (per-user 0700). #[cfg(target_os = "linux")] -pub fn gamescope_ei_socket_file() -> &'static str { - gamescope::EI_SOCKET_FILE +pub fn gamescope_ei_socket_file() -> std::path::PathBuf { + gamescope::ei_socket_file() } /// Call when a client session ends: if the host-managed gamescope path took over a box's autologin diff --git a/crates/punktfunk-host/src/vdisplay/linux/gamescope.rs b/crates/punktfunk-host/src/vdisplay/linux/gamescope.rs index 6c76e94..b9d00b8 100644 --- a/crates/punktfunk-host/src/vdisplay/linux/gamescope.rs +++ b/crates/punktfunk-host/src/vdisplay/linux/gamescope.rs @@ -670,11 +670,11 @@ pub fn start_restore_worker() -> std::sync::Arc<()> { } /// Point the libei injector at the running gamescope's EIS socket (it reads the relay file -/// [`EI_SOCKET_FILE`]). Best-effort — video still works without it (input just won't reach the +/// [`ei_socket_file`]). Best-effort — video still works without it (input just won't reach the /// session). Shared by the attach and host-managed-session paths. fn point_injector_at_eis() { match find_gamescope_eis_socket() { - Some(sock) => match std::fs::write(EI_SOCKET_FILE, &sock) { + Some(sock) => match std::fs::write(ei_socket_file(), &sock) { Ok(()) => { tracing::info!(socket = %sock, "gamescope: pointed injector at the session's EIS socket") } @@ -770,18 +770,31 @@ fn stop_session(unit_name: &str) { let _ = Command::new("systemctl") .args(["--user", "stop", unit_name]) .status(); - let _ = std::fs::remove_file(EI_SOCKET_FILE); + let _ = std::fs::remove_file(ei_socket_file()); } -/// File where the wrapper below writes gamescope's `LIBEI_SOCKET` (its EIS server socket), -/// read by the libei injector to drive input into the nested app. See [`crate::inject`]. -pub const EI_SOCKET_FILE: &str = "/tmp/punktfunk-gamescope-ei"; +/// File where the wrapper below writes gamescope's `LIBEI_SOCKET` (its EIS server socket), read by +/// the libei injector to drive input into the nested app. See [`crate::inject`]. +/// +/// Placed under `$XDG_RUNTIME_DIR` (a per-user, 0700 directory) — NOT a world-writable `/tmp` — +/// so a second unprivileged local user can neither read the relayed socket path nor pre-plant the +/// file to redirect the host's injector to a rogue EIS server (which would let them keylog or deny +/// the remote session's keyboard/mouse input; security-review 2026-06-28 #6). Falls back to `/tmp` +/// only if `XDG_RUNTIME_DIR` is unset (gamescope itself requires it, so this is rare); the reader +/// ([`crate::inject`]) additionally rejects a symlinked relay file as defense-in-depth. +pub fn ei_socket_file() -> std::path::PathBuf { + let runtime = crate::vdisplay::with_env_lock(|| std::env::var_os("XDG_RUNTIME_DIR")); + match runtime { + Some(rt) if !rt.is_empty() => std::path::PathBuf::from(rt).join("punktfunk-gamescope-ei"), + _ => std::path::PathBuf::from("/tmp/punktfunk-gamescope-ei"), + } +} /// Spawn `gamescope --backend headless -W w -H h -r hz -- `. The app comes from /// `PUNKTFUNK_GAMESCOPE_APP` (default a no-op that just keeps gamescope alive — set it to a real /// game/GL app for actual content, e.g. `steam -gamepadui` for the SteamOS-like session). /// stdout/stderr go to `/tmp/punktfunk-gamescope.log`. The app is launched through a tiny shell -/// wrapper that relays gamescope's `LIBEI_SOCKET` (set for its children) to [`EI_SOCKET_FILE`] +/// wrapper that relays gamescope's `LIBEI_SOCKET` (set for its children) to [`ei_socket_file`] /// so the input injector can connect to gamescope's EIS server from outside. fn spawn(w: u32, h: u32, hz: u32, cmd: Option<&str>) -> Result { // A non-empty per-session command (set via `set_launch_command`) wins; else the @@ -791,10 +804,13 @@ fn spawn(w: u32, h: u32, hz: u32, cmd: Option<&str>) -> Result { let app = cmd .map(str::to_string) .filter(|s| !s.trim().is_empty()) - .or_else(|| std::env::var("PUNKTFUNK_GAMESCOPE_APP").ok()) + // Read the env fallback under the shared env lock so it can't race a concurrent session's + // `set_var` of the same key (security-review 2026-06-28 #7). + .or_else(|| crate::vdisplay::with_env_lock(|| std::env::var("PUNKTFUNK_GAMESCOPE_APP").ok())) .filter(|s| !s.trim().is_empty()) .unwrap_or_else(|| "sleep infinity".to_string()); - let _ = std::fs::remove_file(EI_SOCKET_FILE); // stale socket path from a previous session + let relay = ei_socket_file(); + let _ = std::fs::remove_file(&relay); // stale socket path from a previous session let mut cmd = Command::new("gamescope"); cmd.args(["--backend", "headless"]) .args(["-W", &w.to_string()]) @@ -804,7 +820,10 @@ fn spawn(w: u32, h: u32, hz: u32, cmd: Option<&str>) -> Result { .args([ "sh", "-c", - &format!("printf %s \"$LIBEI_SOCKET\" > {EI_SOCKET_FILE}; exec \"$@\""), + &format!( + "printf %s \"$LIBEI_SOCKET\" > '{}'; exec \"$@\"", + relay.display() + ), "sh", ]) .args(app.split_whitespace()) @@ -997,7 +1016,7 @@ impl Drop for GamescopeProc { let _ = self.0.wait(); // Clear the relayed EIS socket name so the host-lifetime injector can't reconnect to this // now-dead session's socket between sessions (the stale path is the "Connection refused"). - let _ = std::fs::remove_file(EI_SOCKET_FILE); + let _ = std::fs::remove_file(ei_socket_file()); } }