From 6cd25b4829f52ef3ba2f09daaf0c5871c84144eb Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 15 Aug 2026 10:50:24 +0200 Subject: [PATCH] fix(host/windows): allow-list host.env keys loaded into the SYSTEM service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/punktfunk-host/src/windows/service.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/punktfunk-host/src/windows/service.rs b/crates/punktfunk-host/src/windows/service.rs index 472db2ca..07a18b1b 100644 --- a/crates/punktfunk-host/src/windows/service.rs +++ b/crates/punktfunk-host/src/windows/service.rs @@ -211,12 +211,24 @@ fn load_host_env() { } if let Some((k, v)) = line.split_once('=') { let (k, v) = (k.trim(), v.trim().trim_matches('"')); - if !k.is_empty() { + // Allow-list, matching `interactive::merged_env_block`'s filter at the child-spawn + // boundary: import ONLY `PUNKTFUNK_*` / `RUST_LOG` into the LocalSystem service's own + // environment. Without this, EVERY key was injected — including `SystemRoot`, from which + // `pf_paths::icacls_path()` / the powershell warner build the absolute program paths a + // privileged service must never resolve through a poisoned env — so a `host.env` planted + // in the user-writable %ProgramData% before install yielded code execution as SYSTEM. + // security-review 2026-08-15 finding 3. (Note: `PUNKTFUNK_HOST_CMD` / `PUNKTFUNK_CONFIG_DIR` + // are legitimate installer-written knobs and still pass; a PLANTED file redirecting THEM + // is closed separately by distrusting a non-admin-owned host.env — findings 3c/4.) + let allowed = k.starts_with("PUNKTFUNK_") || k == "RUST_LOG"; + if !k.is_empty() && allowed { // SAFETY: called from the service main before this process spawns any thread — // the network-profile warner and the supervisor's host child both start after // `load_host_env` returns, so nothing reads the environment concurrently. unsafe { std::env::set_var(k, v) }; n += 1; + } else if !k.is_empty() { + tracing::warn!(key = %k, "host.env: ignoring non-allow-listed key"); } } }