fix(host/windows): allow-list host.env keys loaded into the SYSTEM service
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.
This commit is contained in:
@@ -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");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user