diff options
| -rw-r--r-- | vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch | 220 | ||||
| -rw-r--r-- | vendor/portable-pty.patches.md | 45 | ||||
| -rw-r--r-- | vendor/portable-pty/src/cmdbuilder.rs | 172 |
3 files changed, 405 insertions, 32 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]); ++ } + } diff --git a/vendor/portable-pty.patches.md b/vendor/portable-pty.patches.md index 8ed47a65..14eae869 100644 --- a/vendor/portable-pty.patches.md +++ b/vendor/portable-pty.patches.md @@ -84,3 +84,48 @@ python3 -m unittest scripts.test_vendor_portable_pty ``` On Windows, also run `cargo test raw_arg_appends_unescaped_windows_command_tail`. + +## 0003 reject malformed Windows environments + +status: active + +patch: `vendor/patches/portable-pty/0003-reject-malformed-windows-environments.patch` + +herdr issue: https://github.com/herdrdev/herdr/issues/3430 + +upstream discussions: + +- https://github.com/wezterm/wezterm/issues/4364 +- https://github.com/warpdotdev/warp/commit/2992d02e3e38af697c83a3bd6f20003f54ffe066 + +upstream pr: none + +vendored base: `portable-pty 0.9.0` + +local files: + +- `vendor/portable-pty/src/cmdbuilder.rs` + +reason: Windows process environments may contain a registry value with an +empty name or a non-string type. `winreg 0.10` converts `REG_MULTI_SZ` to an +`OsString` that can contain embedded nulls. `portable-pty` serialized those +values unchanged, producing an invalid environment block that makes +`CreateProcessW` fail with `ERROR_INVALID_PARAMETER` (87). Import only Windows +environment string types and omit entries that cannot form one complete +`name=value\0` record. + +remove when: upstream `portable-pty` both imports only valid Windows environment +string types and prevents malformed names or values from corrupting the process +environment block, or Herdr replaces this launch path. + +verification: + +```sh +python3 -m unittest scripts.test_vendor_portable_pty +``` + +On Windows, also run: + +```sh +cargo test --manifest-path vendor/portable-pty/Cargo.toml windows_environment_rejects_malformed_entries +``` diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs index 385fe81c..a9395f41 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]); + } } |