fix(windows): reject malformed process environments (#3500)

* fix(windows): reject malformed process environments

refs #3430

* fix(windows): validate registry environment strings

refs #3430

* fix(windows): preserve hidden environment variables

refs #3430

1 files changed, 220 insertions, 0 deletions

diff --git a/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch b/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch
new file mode 100644
index 00000000..d2f7c4ae
--- /dev/null
+++ b/vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch
@@ -0,0 +1,220 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: herdr maintainers <maintainers@herdr.dev>
+Date: Tue, 1 Sep 2026 00:00:00 +0000
+Subject: [PATCH] reject malformed Windows process environments
+
+Windows registry environment keys can contain empty names, non-string types,
+or malformed UTF-16. Decode string values into aligned storage, check Win32
+expansion failures, preserve valid drive variables, and omit entries that
+cannot form a valid environment record before calling CreateProcessW.
+
+Herdr issue: https://github.com/herdrdev/herdr/issues/3430
+Vendored base: portable-pty 0.9.0
+---
+diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs
+index 385fe81c..b214bb6d 100644
+--- a/vendor/portable-pty/src/cmdbuilder.rs
++++ b/vendor/portable-pty/src/cmdbuilder.rs
+@@ -71,6 +71,70 @@ fn get_shell() -> String {
+     "/bin/sh".into()
+ }
+ 
++#[cfg(windows)]
++fn reg_value_to_string(value: &winreg::RegValue) -> anyhow::Result<OsString> {
++    use std::os::windows::ffi::OsStringExt;
++    use winapi::um::processenv::ExpandEnvironmentStringsW;
++    use winreg::enums::RegType;
++
++    if !matches!(value.vtype, RegType::REG_SZ | RegType::REG_EXPAND_SZ) {
++        anyhow::bail!(
++            "unsupported registry type {:?} for environment variable",
++            value.vtype
++        );
++    }
++
++    let mut bytes = value.bytes.chunks_exact(2);
++    let mut units: Vec<u16> = bytes
++        .by_ref()
++        .map(|pair| u16::from_le_bytes([pair[0], pair[1]]))
++        .collect();
++    anyhow::ensure!(
++        bytes.remainder().is_empty(),
++        "invalid UTF-16 byte length for environment variable"
++    );
++    while let Some(0) = units.last() {
++        units.pop();
++    }
++    anyhow::ensure!(
++        !units.contains(&0),
++        "embedded null in registry environment variable"
++    );
++
++    if value.vtype == RegType::REG_SZ {
++        return Ok(OsString::from_wide(&units));
++    }
++
++    units.push(0);
++    let size = unsafe { ExpandEnvironmentStringsW(units.as_ptr(), std::ptr::null_mut(), 0) };
++    if size == 0 {
++        anyhow::bail!(
++            "ExpandEnvironmentStringsW failed: {}",
++            std::io::Error::last_os_error()
++        );
++    }
++    let mut buf = vec![0u16; size as usize];
++    let written = unsafe {
++        ExpandEnvironmentStringsW(units.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
++    };
++    if written == 0 {
++        anyhow::bail!(
++            "ExpandEnvironmentStringsW failed: {}",
++            std::io::Error::last_os_error()
++        );
++    }
++    anyhow::ensure!(
++        written as usize <= buf.len(),
++        "environment changed while expanding registry value"
++    );
++
++    buf.truncate(written as usize);
++    while let Some(0) = buf.last() {
++        buf.pop();
++    }
++    Ok(OsString::from_wide(&buf))
++}
++
+ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
+     let mut env: BTreeMap<OsString, EnvEntry> = std::env::vars_os()
+         .map(|(key, value)| {
+@@ -103,37 +167,8 @@ fn get_base_env() -> BTreeMap<OsString, EnvEntry> {
+ 
+     #[cfg(windows)]
+     {
+-        use std::os::windows::ffi::OsStringExt;
+-        use winapi::um::processenv::ExpandEnvironmentStringsW;
+-        use winreg::enums::{RegType, HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
+-        use winreg::types::FromRegValue;
+-        use winreg::{RegKey, RegValue};
+-
+-        fn reg_value_to_string(value: &RegValue) -> anyhow::Result<OsString> {
+-            match value.vtype {
+-                RegType::REG_EXPAND_SZ => {
+-                    let src = unsafe {
+-                        std::slice::from_raw_parts(
+-                            value.bytes.as_ptr() as *const u16,
+-                            value.bytes.len() / 2,
+-                        )
+-                    };
+-                    let size =
+-                        unsafe { ExpandEnvironmentStringsW(src.as_ptr(), std::ptr::null_mut(), 0) };
+-                    let mut buf = vec![0u16; size as usize + 1];
+-                    unsafe {
+-                        ExpandEnvironmentStringsW(src.as_ptr(), buf.as_mut_ptr(), buf.len() as u32)
+-                    };
+-
+-                    let mut buf = buf.as_slice();
+-                    while let Some(0) = buf.last() {
+-                        buf = &buf[0..buf.len() - 1];
+-                    }
+-                    Ok(OsString::from_wide(buf))
+-                }
+-                _ => Ok(OsString::from_reg_value(value)?),
+-            }
+-        }
++        use winreg::enums::{HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
++        use winreg::RegKey;
+ 
+         if let Ok(sys_env) = RegKey::predef(HKEY_LOCAL_MACHINE)
+             .open_subkey("System\\CurrentControlSet\\Control\\Session Manager\\Environment")
+@@ -662,12 +697,24 @@ impl CommandBuilder {
+             value,
+         } in self.envs.values()
+         {
+-            block.extend(preferred_key.encode_wide());
++            let key: Vec<u16> = preferred_key.encode_wide().collect();
++            if key.is_empty()
++                || key.contains(&0)
++                || key[1..].contains(&(b'=' as u16))
++                || value.encode_wide().any(|unit| unit == 0)
++            {
++                continue;
++            }
++
++            block.extend(key);
+             block.push(b'=' as u16);
+             block.extend(value.encode_wide());
+             block.push(0);
+         }
+         // and a final terminator for CreateProcessW
++        if block.is_empty() {
++            block.push(0);
++        }
+         block.push(0);
+ 
+         block
+@@ -869,4 +916,65 @@ mod tests {
+         assert!(command.ends_with(r#"/d /c echo "hi""#), "{}", command);
+         assert!(!command.contains(r#"\"hi\""#), "{}", command);
+     }
++
++    #[cfg(windows)]
++    #[test]
++    fn windows_environment_rejects_malformed_entries() {
++        use std::os::windows::ffi::{OsStrExt, OsStringExt};
++        use winreg::enums::RegType;
++        use winreg::RegValue;
++
++        let multi_string = RegValue {
++            bytes: vec![b'a', 0, 0, 0],
++            vtype: RegType::REG_MULTI_SZ,
++        };
++        assert!(reg_value_to_string(&multi_string).is_err());
++        let malformed_expand_string = RegValue {
++            bytes: vec![b'%', 0, b'P'],
++            vtype: RegType::REG_EXPAND_SZ,
++        };
++        assert!(reg_value_to_string(&malformed_expand_string).is_err());
++        let expandable_string = RegValue {
++            bytes: "literal\0"
++                .encode_utf16()
++                .flat_map(u16::to_le_bytes)
++                .collect(),
++            vtype: RegType::REG_EXPAND_SZ,
++        };
++        assert_eq!(
++            reg_value_to_string(&expandable_string).unwrap(),
++            OsString::from("literal")
++        );
++
++        let mut cmd = CommandBuilder::new("dummy");
++        cmd.env_clear();
++        cmd.env("", "ignored");
++        cmd.env(
++            OsString::from_wide(&[b'B' as u16, 0, b'A' as u16, b'D' as u16]),
++            "ignored",
++        );
++        cmd.env(
++            "MULTI",
++            OsString::from_wide(&[b'a' as u16, 0, b'b' as u16]),
++        );
++        cmd.env("BAD=KEY", "ignored");
++        cmd.env("=::", "colon");
++        cmd.env("=C:", r"C:\valid");
++        cmd.env("=ExitCode", "hidden");
++        cmd.env("EMPTY", "");
++        cmd.env("VALID", "ok");
++
++        let expected: Vec<u16> =
++            OsStr::new("=::=colon\0=C:=C:\\valid\0=ExitCode=hidden\0EMPTY=\0VALID=ok\0\0")
++                .encode_wide()
++                .collect();
++        assert_eq!(cmd.environment_block(), expected);
++
++        cmd.env_remove("=::");
++        cmd.env_remove("=C:");
++        cmd.env_remove("=ExitCode");
++        cmd.env_remove("EMPTY");
++        cmd.env_remove("VALID");
++        assert_eq!(cmd.environment_block(), vec![0, 0]);
++    }
+ }