From e707a962b6a371903ed4f77d12dc1546bcfcb142 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 10 Jul 2026 10:04:23 +0200 Subject: [PATCH] fix(security): anti-replay, 0600 client key, open redirect, supply-chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address findings from a repo security review: - core: add a sliding-window anti-replay filter over the AEAD-authenticated sequence in Session (poll_input/poll_frame), closing the input-replay gap the data plane previously left to the LAN/VPN trust assumption. 4096-deep window, unit-tested; the encrypted loopback suite confirms no false drops. - clients: write the mTLS client private key 0600 and lock the config dir 0700 on Unix (it was world-readable at the umask default), re-locking existing stores on load. pf-client-core::trust plus the probe's own identity writer. Windows keeps the %APPDATA% ACL; Android/Apple already wrap the key. - web: fix a post-login open redirect — resolve `next` via URL and require it to stay same-origin, rejecting `/\evil.com` and tab/encoding variants the old `!startsWith("//")` guard missed. Also fixes the dead safeNextPath helper. - ci: SHA-256-pin the BtbN FFmpeg DLLs bundled into the signed Windows installer (were fetched from the rolling `latest` tag unverified); fails closed on a re-roll, matching the VB-CABLE gate. - ci: fail-open fork-guard on the Windows/Apple host-mode PR build jobs that share runner labels with the signing jobs. Definitive fix stays server-side (Gitea outside-collaborator approval / isolated PR runners) — see the notes. Co-Authored-By: Claude Opus 4.8 --- .gitea/workflows/apple.yml | 9 + .gitea/workflows/windows-drivers.yml | 10 + .gitea/workflows/windows.yml | 12 ++ clients/probe/src/main.rs | 25 ++- crates/pf-client-core/src/trust.rs | 47 +++- crates/punktfunk-core/src/session.rs | 200 +++++++++++++++++- .../ci/provision-windows-punktfunk-extras.ps1 | 20 +- web/server/util/auth.ts | 16 +- web/src/sections/Login/index.tsx | 17 +- 9 files changed, 338 insertions(+), 18 deletions(-) diff --git a/.gitea/workflows/apple.yml b/.gitea/workflows/apple.yml index 209e0b49..b1878bb7 100644 --- a/.gitea/workflows/apple.yml +++ b/.gitea/workflows/apple.yml @@ -16,8 +16,17 @@ on: workflow_dispatch: jobs: + # SECURITY: builds/tests PULL-REQUEST code on the host-mode, persistent `macos-arm64` runner shared + # with the release-signing job (release.yml, which loads the App Store Connect key). Untrusted PR + # code could persist on it or harvest signing material. Definitive fix is server-side: enable Gitea's + # "require approval for PRs from outside collaborators/forks", and/or isolate PR CI on ephemeral + # runners. The `if:` is a fail-open backstop — it skips fork PRs where Gitea reports the fork flag and + # still runs same-repo PRs (and where the flag is absent), so it never blocks internal PR CI. swift: runs-on: macos-arm64 + if: >- + gitea.event_name != 'pull_request' || + gitea.event.pull_request.head.repo.fork != true timeout-minutes: 60 steps: - uses: actions/checkout@v4 diff --git a/.gitea/workflows/windows-drivers.yml b/.gitea/workflows/windows-drivers.yml index 315c8f6d..45cc2542 100644 --- a/.gitea/workflows/windows-drivers.yml +++ b/.gitea/workflows/windows-drivers.yml @@ -30,8 +30,15 @@ on: # scripts/ci/ensure-windows-toolchain.ps1, a fast no-op once already present. jobs: + # SECURITY: builds PULL-REQUEST code on the host-mode, persistent `windows-amd64` runner shared with + # the release-signing jobs (windows-host.yml / windows-msix.yml). See windows.yml for the full + # rationale. Definitive fix is server-side (Gitea outside-collaborator approval + isolated PR + # runners); the `if:` is a fail-open backstop that never blocks internal PR CI. probe-and-proto: runs-on: windows-amd64 + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.fork != true timeout-minutes: 30 defaults: run: @@ -108,6 +115,9 @@ jobs: # DLL's FORCE_INTEGRITY (/INTEGRITYCHECK) bit — the M0 self-signed-load question. driver-build: runs-on: windows-amd64 + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.fork != true timeout-minutes: 45 defaults: run: diff --git a/.gitea/workflows/windows.yml b/.gitea/workflows/windows.yml index cf2cb8e4..00e059e0 100644 --- a/.gitea/workflows/windows.yml +++ b/.gitea/workflows/windows.yml @@ -68,8 +68,20 @@ on: workflow_dispatch: jobs: + # SECURITY: this job builds PULL-REQUEST code (attacker-controllable build.rs / cargo build) on the + # host-mode, persistent `windows-amd64` runner that the release-SIGNING jobs (windows-host.yml / + # windows-msix.yml, which decrypt MSIX_CERT_PFX_B64 + REGISTRY_TOKEN to disk) also run on. Untrusted + # PR code could therefore persist on that machine or harvest signing material a later job exposes. + # The DEFINITIVE fix is operational and lives outside this file: enable Gitea's "require approval to + # run workflows for PRs from outside collaborators/forks", and/or route PR CI to isolated ephemeral + # runners. The `if:` below is only a backstop — it skips fork PRs where Gitea reports the fork flag, + # and FAILS OPEN (still runs) for same-repo PRs and on Gitea versions that don't populate it, so it + # never blocks internal PR CI. build: runs-on: windows-amd64 + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.fork != true timeout-minutes: 90 strategy: fail-fast: false diff --git a/clients/probe/src/main.rs b/clients/probe/src/main.rs index 47be77fd..8579d4c6 100644 --- a/clients/probe/src/main.rs +++ b/clients/probe/src/main.rs @@ -145,11 +145,34 @@ fn load_or_create_identity() -> Result<(String, String)> { let dir = std::path::PathBuf::from(home).join(".config/punktfunk"); let (cp, kp) = (dir.join("client-cert.pem"), dir.join("client-key.pem")); if let (Ok(c), Ok(k)) = (std::fs::read_to_string(&cp), std::fs::read_to_string(&kp)) { + // Re-lock a store an older build left world-readable (this key is shared with the other + // clients' `~/.config/punktfunk/client-key.pem`); best-effort. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); + let _ = std::fs::set_permissions(&kp, std::fs::Permissions::from_mode(0o600)); + } return Ok((c, k)); } let (c, k) = endpoint::generate_identity().map_err(|e| anyhow!("generate identity: {e}"))?; std::fs::create_dir_all(&dir)?; - std::fs::write(&cp, &c)?; + std::fs::write(&cp, &c)?; // the certificate is public + // The key is the mTLS credential a paired host authorizes for full remote control, so it must + // not be world-readable — create it 0600 (a plain `fs::write` honors the umask → typically 0644). + #[cfg(unix)] + { + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); + let mut f = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(&kp)?; + f.write_all(k.as_bytes())?; + } + #[cfg(not(unix))] std::fs::write(&kp, &k)?; tracing::info!(cert = %cp.display(), "generated client identity"); Ok((c, k)) diff --git a/crates/pf-client-core/src/trust.rs b/crates/pf-client-core/src/trust.rs index ec4a12bc..f25e0993 100644 --- a/crates/pf-client-core/src/trust.rs +++ b/crates/pf-client-core/src/trust.rs @@ -34,16 +34,61 @@ pub fn load_or_create_identity() -> Result<(String, String)> { let dir = config_dir()?; let (cp, kp) = (dir.join("client-cert.pem"), dir.join("client-key.pem")); if let (Ok(c), Ok(k)) = (std::fs::read_to_string(&cp), std::fs::read_to_string(&kp)) { + // An older build wrote the key with a plain `fs::write`, which honors the umask and + // typically lands 0644 — world-readable. Re-lock an existing store on load so upgrades + // get fixed, not just fresh installs. Best-effort (a read-only store keeps what it has). + #[cfg(unix)] + lock_identity_perms(&dir, &kp); return Ok((c, k)); } let (c, k) = endpoint::generate_identity().map_err(|e| anyhow!("generate identity: {e}"))?; std::fs::create_dir_all(&dir)?; + // The private key authorizes this client for full remote control of a paired host, so it must + // never be world-readable: lock the dir to the owner (0700) and create the key 0600 from the + // start (`fs::write` alone honors the umask → typically 0644). The certificate is public. On + // non-Unix the %APPDATA% profile ACL already scopes the dir to the user, so std perms suffice. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700))?; + } std::fs::write(&cp, &c)?; - std::fs::write(&kp, &k)?; + write_private_key(&kp, k.as_bytes())?; tracing::info!(cert = %cp.display(), "generated client identity"); Ok((c, k)) } +/// Write the client's mTLS private key owner-only. On Unix the file is created with mode 0600 from +/// the outset — an `fs::write` + later `chmod` would briefly expose it at the umask default. On +/// other platforms std's default perms plus the %APPDATA% profile ACL scope it to the user. +fn write_private_key(path: &std::path::Path, bytes: &[u8]) -> Result<()> { + #[cfg(unix)] + { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + let mut f = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path)?; + f.write_all(bytes)?; + } + #[cfg(not(unix))] + std::fs::write(path, bytes)?; + Ok(()) +} + +/// Best-effort re-lock of an already-present identity (dir 0700, key 0600) — for stores written by +/// an older build that left the key world-readable. Errors are ignored: the worst case is the +/// pre-existing perms, which this never loosens. +#[cfg(unix)] +fn lock_identity_perms(dir: &std::path::Path, key: &std::path::Path) { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700)); + let _ = std::fs::set_permissions(key, std::fs::Permissions::from_mode(0o600)); +} + pub fn hex(fp: &[u8; 32]) -> String { fp.iter().map(|b| format!("{b:02x}")).collect() } diff --git a/crates/punktfunk-core/src/session.rs b/crates/punktfunk-core/src/session.rs index feafa28f..93662874 100644 --- a/crates/punktfunk-core/src/session.rs +++ b/crates/punktfunk-core/src/session.rs @@ -28,15 +28,18 @@ pub struct Frame { /// One end of a stream. Constructed for a single [`Role`]; calling the other role's /// methods returns [`PunktfunkError::InvalidArg`]. /// -/// Note: the AEAD layer authenticates each datagram but does **not** provide anti-replay. -/// Video replays are largely absorbed by the reassembler's per-frame dedup, but replayed -/// input events are not yet filtered. A sliding-window replay filter keyed on the -/// authenticated sequence belongs with the pairing/handshake layer (the GameStream host); until then, -/// rely on the LAN/VPN transport assumption (plan §1). +/// Anti-replay: the receive path runs each opened datagram's AEAD-authenticated sequence through a +/// sliding-window filter ([`ReplayWindow`]), so a captured, validly-sealed datagram can't be replayed +/// by an on-path attacker — closing the input-replay gap that previously rested solely on the +/// LAN/VPN transport assumption (plan §1). Genuine reordering within the window is still accepted; +/// video additionally benefits from the reassembler's per-frame dedup. pub struct Session { config: Config, coder: Box, crypto: Option, + /// Anti-replay window over the peer's authenticated sequence (receive side). `Some` exactly when + /// `crypto` is — the plaintext probe path carries no sequence to filter on. + replay: Option, transport: Box, packetizer: Packetizer, reassembler: Reassembler, @@ -68,11 +71,15 @@ impl Session { let crypto = config .encrypt .then(|| SessionCrypto::new(&config.key, config.salt, config.role)); + // A receive-side replay window exists exactly when the datagrams are sealed (they carry the + // authenticated sequence the window keys on). Both roles receive from their peer. + let replay = config.encrypt.then(ReplayWindow::new); let packetizer = Packetizer::new(&config); let reassembler = Reassembler::new(ReassemblerLimits::from_config(&config)); Ok(Session { coder, crypto, + replay, transport, packetizer, reassembler, @@ -130,6 +137,16 @@ impl Session { } } + /// Feed an opened datagram's authenticated sequence to the anti-replay window: `true` = fresh + /// (accept), `false` = a replay or older than the window (drop). Returns `true` when the session + /// isn't encrypting (no window, and no sequence on the wire to key on). + fn accept_seq(&mut self, seq: u64) -> bool { + match self.replay.as_mut() { + Some(w) => w.accept(seq), + None => true, + } + } + // -- Host path -------------------------------------------------------- /// Host: FEC-protect, packetize, and seal one encoded access unit into wire packets WITHOUT @@ -225,6 +242,13 @@ impl Session { Ok(p) => p, Err(_) => continue, // drop undecryptable noise }; + // Anti-replay: a captured input datagram replayed by an on-path attacker opens cleanly + // (its sequence + tag are still valid) — the window is what rejects the second copy. + // `len >= 8` is guaranteed because the sealed-path open above succeeded. + if self.replay.is_some() && !self.accept_seq(seq_of(&wire)) { + StatsCounters::add(&self.stats.packets_dropped, 1); + continue; + } StatsCounters::add(&self.stats.packets_received, 1); if let Some(ev) = InputEvent::decode(&pkt) { return Ok(Some(ev)); @@ -276,6 +300,13 @@ impl Session { Ok(p) => p, Err(_) => continue, }; + // Anti-replay (same rationale as poll_input): reject a datagram whose authenticated + // sequence was already seen. Video also dedups per-frame downstream, but filtering here + // is uniform and cheap. `len >= 8` because the sealed-path open above succeeded. + if self.replay.is_some() && !self.accept_seq(seq_of(&self.recv_scratch[i][..len])) { + StatsCounters::add(&self.stats.packets_dropped, 1); + continue; + } StatsCounters::add(&self.stats.packets_received, 1); StatsCounters::add(&self.stats.bytes_received, pkt.len() as u64); // The reassembler validates the packet via its parsed header (`magic`), @@ -347,3 +378,162 @@ impl Session { Ok(()) } } + +/// Extract the AEAD-authenticated 8-byte big-endian sequence prefix from a sealed wire datagram. +/// Only called on the encrypted receive path, where a preceding successful open has already +/// established `wire.len() >= 8`. +fn seq_of(wire: &[u8]) -> u64 { + u64::from_be_bytes(wire[..8].try_into().unwrap()) +} + +/// Depth of the anti-replay window, in sequences. The sender advances its sequence once per +/// datagram, so at the data plane's packet rate 4096 is roughly 33 ms of reorder tolerance for the +/// video stream (well beyond any reordering still useful for a live frame) and effectively unbounded +/// for the sparse input stream — while bounding how far back a replay could hide. +const REPLAY_WINDOW: u64 = 4096; +const REPLAY_WORDS: usize = (REPLAY_WINDOW / 64) as usize; + +/// Sliding-window anti-replay filter over the AEAD-authenticated wire sequence. The sender counts +/// its datagrams from 0, and the protocol never legitimately re-sends a sequence (FEC recovery +/// shards get fresh ones), so a sequence seen twice is a replay. The AEAD tag already authenticates +/// the sequence — a forged one can't open — so this only has to reject *duplicates* of validly +/// sealed datagrams (and anything older than the window, which we can no longer prove is fresh). +/// Genuine reordering within the window is accepted. Bitmap-per-sequence, indexed `seq % WINDOW`. +struct ReplayWindow { + /// Highest sequence accepted so far; `seen` stays false until the first datagram. + highest: u64, + seen: bool, + /// One bit per in-window sequence in `(highest - WINDOW, highest]`. + bits: [u64; REPLAY_WORDS], +} + +impl ReplayWindow { + fn new() -> ReplayWindow { + ReplayWindow { + highest: 0, + seen: false, + bits: [0; REPLAY_WORDS], + } + } + + #[inline] + fn word_bit(seq: u64) -> (usize, u64) { + let idx = (seq % REPLAY_WINDOW) as usize; + (idx / 64, 1u64 << (idx % 64)) + } + fn is_set(&self, seq: u64) -> bool { + let (w, b) = Self::word_bit(seq); + self.bits[w] & b != 0 + } + fn set(&mut self, seq: u64) { + let (w, b) = Self::word_bit(seq); + self.bits[w] |= b; + } + fn unset(&mut self, seq: u64) { + let (w, b) = Self::word_bit(seq); + self.bits[w] &= !b; + } + + /// Record `seq`, returning `true` if it's fresh (accept) or `false` if it's a replay / too old. + fn accept(&mut self, seq: u64) -> bool { + if !self.seen { + self.seen = true; + self.highest = seq; + self.set(seq); + return true; + } + if seq > self.highest { + // Advance the window. Sequences between the old and new high slide in unseen, so clear + // their (possibly stale, from a full window ago) slots — unless we jumped an entire + // window, in which case wipe the bitmap wholesale. + if seq - self.highest >= REPLAY_WINDOW { + self.bits = [0; REPLAY_WORDS]; + } else { + let mut s = self.highest + 1; + while s < seq { + self.unset(s); + s += 1; + } + } + self.highest = seq; + self.set(seq); + true + } else if self.highest - seq >= REPLAY_WINDOW || self.is_set(seq) { + // Older than the window (can't prove it isn't a replay) or already seen (a duplicate) — + // either way, drop it. + false + } else { + self.set(seq); // in-window and not yet seen — a genuine reorder + true + } + } +} + +#[cfg(test)] +mod replay_tests { + use super::*; + + #[test] + fn accepts_in_order_and_rejects_duplicates() { + let mut w = ReplayWindow::new(); + for seq in 0..1000 { + assert!(w.accept(seq), "fresh in-order seq {seq} must be accepted"); + } + // Every one of those is now a replay. + for seq in 0..1000 { + assert!(!w.accept(seq), "replayed seq {seq} must be rejected"); + } + } + + #[test] + fn accepts_reorder_within_window_once() { + let mut w = ReplayWindow::new(); + assert!(w.accept(100)); + // Earlier-but-in-window sequences (a genuine reorder) are accepted exactly once. + assert!(w.accept(80)); + assert!(!w.accept(80), "second copy of a reordered seq is a replay"); + assert!(w.accept(99)); + assert!(!w.accept(100), "the high-water seq itself can't be replayed"); + } + + #[test] + fn rejects_older_than_window() { + let mut w = ReplayWindow::new(); + assert!(w.accept(REPLAY_WINDOW * 2)); + // Anything a full window or more behind the high-water mark is dropped (can't prove fresh). + assert!(!w.accept(REPLAY_WINDOW * 2 - REPLAY_WINDOW)); + assert!(!w.accept(0)); + // But just inside the window is still accepted. + assert!(w.accept(REPLAY_WINDOW * 2 - (REPLAY_WINDOW - 1))); + } + + #[test] + fn large_forward_jump_wipes_stale_bits() { + let mut w = ReplayWindow::new(); + assert!(w.accept(5)); + // Jump far forward (more than a window). The slot for an old seq that aliases 5 mod WINDOW + // must read as unseen afterward, i.e. the jump cleared it — so a NEW seq there is accepted. + let far = 10 * REPLAY_WINDOW + 5; + assert!(w.accept(far)); + assert!(!w.accept(5), "the pre-jump seq is now far older than the window"); + // A fresh seq aliasing 5 (mod WINDOW) but inside the new window is accepted, proving the + // stale bit was cleared rather than mistaken for a replay. + assert!(w.accept(far - REPLAY_WINDOW + 1)); + } + + #[test] + fn first_seq_need_not_be_zero() { + // Startup loss can mean the first datagram we ever open isn't seq 0. + let mut w = ReplayWindow::new(); + assert!(w.accept(42)); + assert!(!w.accept(42)); + assert!(w.accept(43)); + } + + #[test] + fn seq_of_reads_the_big_endian_prefix() { + let mut wire = 0x0102_0304_0506_0708u64.to_be_bytes().to_vec(); + wire.extend_from_slice(b"ciphertext-and-tag"); + assert_eq!(seq_of(&wire), 0x0102_0304_0506_0708); + } +} diff --git a/scripts/ci/provision-windows-punktfunk-extras.ps1 b/scripts/ci/provision-windows-punktfunk-extras.ps1 index cf38a173..4367316d 100644 --- a/scripts/ci/provision-windows-punktfunk-extras.ps1 +++ b/scripts/ci/provision-windows-punktfunk-extras.ps1 @@ -34,13 +34,25 @@ if (Test-Path $rustup) { # shipped installer/MSIX stay consistent with punktfunk's MIT OR Apache-2.0 posture. # MIGRATION: a runner previously provisioned with the old *gpl-shared* trees must be # re-provisioned - delete C:\Users\Public\ffmpeg and C:\Users\Public\ffmpeg-arm64, then re-run. +# These DLLs are bundled verbatim into the code-signed host installer/MSIX, so the download is +# SHA-256-pinned (like VB-CABLE below): BtbN's `latest` tag is a ROLLING release whose assets are +# re-uploaded over time, so an unverified fetch would let a hijacked/MITM'd upstream asset land +# signed DLLs in users' installs. The pins below were captured 2026-07-10 from the then-current +# n7.1 lgpl-shared build. When BtbN re-rolls `latest`, this fetch FAILS CLOSED (hash mismatch) — +# that is intentional: re-download, re-verify the new archive, and update the two pins here. +# Refresh a pin: (Get-FileHash .\ffmpeg-.zip -Algorithm SHA256).Hash function Get-BtbnFfmpeg { - param([string]$Dir, [string]$ZipTag) # ZipTag: 'win64' (x64) or 'winarm64' (ARM64 cross tree) + param([string]$Dir, [string]$ZipTag, [string]$Sha) # ZipTag: 'win64' (x64) or 'winarm64' (ARM64 cross tree) if (Test-Path (Join-Path $Dir 'lib\avcodec.lib')) { info "FFmpeg ($ZipTag) already present at $Dir"; return } - info "fetching FFmpeg ($ZipTag, BtbN lgpl-shared)" + info "fetching FFmpeg ($ZipTag, BtbN lgpl-shared, SHA-256 pinned)" $url = "https://github.com/BtbN/FFmpeg-Builds/releases/download/latest/ffmpeg-n7.1-latest-$ZipTag-lgpl-shared-7.1.zip" $zip = "$Dir.zip"; $tmp = "$Dir-extract" Invoke-WebRequest -Uri $url -OutFile $zip -UseBasicParsing + $got = (Get-FileHash $zip -Algorithm SHA256).Hash + if ($got -ne $Sha) { + Remove-Item $zip -Force + throw "FFmpeg ($ZipTag) download hash mismatch (got $got, pinned $Sha). BtbN re-rolled the 'latest' build; re-verify the new archive and update the pinned SHA in this script before shipping." + } if (Test-Path $tmp) { Remove-Item -Recurse -Force $tmp } Expand-Archive -Path $zip -DestinationPath $tmp -Force # BtbN zips have one top-level folder $inner = Get-ChildItem $tmp -Directory | Select-Object -First 1 @@ -48,8 +60,8 @@ function Get-BtbnFfmpeg { Move-Item -Path $inner.FullName -Destination $Dir Remove-Item -Force $zip; Remove-Item -Recurse -Force $tmp -ErrorAction SilentlyContinue } -Get-BtbnFfmpeg -Dir "C:\Users\Public\ffmpeg" -ZipTag 'win64' -Get-BtbnFfmpeg -Dir "C:\Users\Public\ffmpeg-arm64" -ZipTag 'winarm64' +Get-BtbnFfmpeg -Dir "C:\Users\Public\ffmpeg" -ZipTag 'win64' -Sha '89F3469706E5D53AEA5CF34AEE63E62CE746E6159D7AEE473D330B02A47558E6' +Get-BtbnFfmpeg -Dir "C:\Users\Public\ffmpeg-arm64" -ZipTag 'winarm64' -Sha 'D96B4CE08CEBDCC6AD0E3934A3F962915E440EEFB9D73831AFEA4D80E35129A5' # --- Vulkan-Headers (pf-ffvk's bindgen: libavutil/hwcontext_vulkan.h includes , # and Windows has no system copy). Headers only - the loader (vulkan-1.dll) is a GPU-driver diff --git a/web/server/util/auth.ts b/web/server/util/auth.ts index ca3e86ac..3293cb02 100644 --- a/web/server/util/auth.ts +++ b/web/server/util/auth.ts @@ -121,11 +121,19 @@ export function isPublicPath(pathname: string): boolean { return false; } -/** Validate a post-login redirect target: a same-origin path only. Rejects protocol- - * relative (`//evil.com`) and absolute URLs to prevent an open redirect. */ +/** Validate a post-login redirect target: a same-origin path only. Resolves `next` against a + * sentinel origin and keeps it only if it stays same-origin — rejecting absolute (`https://evil.com`), + * protocol-relative (`//evil.com`) AND backslash/tab variants (`/\evil.com`, which the WHATWG URL + * parser folds to `//evil.com`) that a plain `startsWith("//")` guard lets through. */ export function safeNextPath(next: string | undefined): string { - if (!next?.startsWith("/") || next.startsWith("//")) return "/"; - return next; + if (!next) return "/"; + try { + const base = "http://pf.invalid"; + const u = new URL(next, base); + return u.origin === base ? u.pathname + u.search + u.hash : "/"; + } catch { + return "/"; + } } export interface SessionData { diff --git a/web/src/sections/Login/index.tsx b/web/src/sections/Login/index.tsx index d84d6fea..b578b583 100644 --- a/web/src/sections/Login/index.tsx +++ b/web/src/sections/Login/index.tsx @@ -21,9 +21,20 @@ export const SectionLogin: FC<{ next?: string }> = ({ next }) => { setBusy(false); return; } - // Full reload to the target so SSR re-runs WITH the new session cookie. Only a - // same-origin path — reject protocol-relative/absolute URLs (open-redirect guard). - const safe = next?.startsWith("/") && !next.startsWith("//") ? next : "/"; + // Full reload to the target so SSR re-runs WITH the new session cookie. Resolve `next` + // against our own origin and accept it ONLY if it stays same-origin, rejecting absolute + // and protocol-relative targets. A naive `!startsWith("//")` check misses `/\evil.com` + // (browsers fold `\`→`/` under http(s), so it resolves to //evil.com) plus tab/newline + // tricks; parsing closes them because it's the same parser this redirect will use. + let safe = "/"; + try { + const u = new URL(next ?? "/", window.location.origin); + if (u.origin === window.location.origin) { + safe = u.pathname + u.search + u.hash; + } + } catch { + safe = "/"; + } window.location.href = safe; } catch { setError(true);