diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index 54710467c..4cba8e357 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -1,10 +1,6 @@ use rustpython_wtf8::Wtf8; -use std::{ - ffi::{OsStr, OsString}, - io, - os::windows::ffi::OsStringExt, -}; -use widestring::WideCString; +use std::{ffi::OsStr, io}; +use widestring::{WideCStr, WideCString}; use windows_sys::{ Win32::{ Foundation::{ @@ -28,6 +24,8 @@ use windows_sys::{ w, }; +const NUL_ERROR: &str = "embedded null character"; + /// _MAX_ENV from Windows CRT stdlib.h - maximum environment variable size pub const _MAX_ENV: usize = 32767; pub const HRESULT_E_POINTER: i32 = E_POINTER; @@ -218,18 +216,12 @@ pub fn get_windows_version() -> io::Result { } .check_win32_bool()?; - let service_pack = { - let (last, _) = version - .szCSDVersion - .iter() - .take_while(|&x| x != &0) - .enumerate() - .last() - .unwrap_or((0, &0)); - let sp = OsString::from_wide(&version.szCSDVersion[..last]); - sp.into_string() - .map_err(|_| io::Error::other("service pack is not ASCII"))? - }; + // szCSDVersion is at most 128 wide chars and represented as slice so overflow is impossible. + let service_pack = WideCStr::from_slice_truncate(&version.szCSDVersion) + .map_err(|_| io::Error::other(NUL_ERROR))? + .to_string() + .map_err(|_| io::Error::other("service pack is not ASCII"))?; + let (major, minor, build) = get_kernel32_version()?; Ok(WindowsVersionInfo { major, @@ -413,7 +405,7 @@ where T: AsRef, { fn to_wide_cstring(&self) -> Result { - WideCString::from_os_str(self).map_err(|_| io::Error::other("embedded null character")) + WideCString::from_os_str(self).map_err(|_| io::Error::other(NUL_ERROR)) } } @@ -426,11 +418,11 @@ impl ToWideString for Wtf8 { // // For the sake of that behavior, fail on trailing NUL. if self.as_bytes().last().is_some_and(|&b| b == 0) { - return Err(io::Error::other("embedded null character")); + return Err(io::Error::other(NUL_ERROR)); } let mut buf = Vec::with_capacity(self.len() + 1); buf.extend(self.encode_wide()); - WideCString::from_vec(buf).map_err(|_| io::Error::other("embedded null character")) + WideCString::from_vec(buf).map_err(|_| io::Error::other(NUL_ERROR)) } } diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs index a269646e5..746726b77 100644 --- a/crates/vm/src/stdlib/nt.rs +++ b/crates/vm/src/stdlib/nt.rs @@ -10,7 +10,7 @@ pub(crate) mod module { builtins::{PyBytes, PyCapsule, PyDictRef, PyListRef, PyStr, PyStrRef, PyTupleRef}, convert::ToPyException, exceptions::{self, OSErrorBuilder, ToOSErrorBuilder}, - function::{ArgMapping, Either, OptionalArg}, + function::{ArgMapping, Either, FsPath, OptionalArg}, host_env::crt_fd, ospath::{OsPath, OsPathOrFd}, stdlib::os::{_os, DirFd, SupportFunc, SymlinkArgs}, @@ -418,8 +418,6 @@ pub(crate) mod module { argv: Either, vm: &VirtualMachine, ) -> PyResult { - use crate::function::FsPath; - let path = path.to_wide_cstring(vm)?; let argv = vm.extract_elements_with(argv.as_ref(), |obj| { @@ -448,8 +446,6 @@ pub(crate) mod module { env: PyDictRef, vm: &VirtualMachine, ) -> PyResult { - use crate::function::FsPath; - let path = path.to_wide_cstring(vm)?; let argv = vm.extract_elements_with(argv.as_ref(), |obj| { @@ -466,7 +462,7 @@ pub(crate) mod module { } // Build environment strings as "KEY=VALUE\0" wide strings - let mut env_strings: Vec = Vec::new(); + let mut env_strings: Vec = Vec::with_capacity(env.size().used); for (key, value) in env { let key = FsPath::try_from_path_like(key, true, vm)?; let value = FsPath::try_from_path_like(value, true, vm)?; @@ -482,7 +478,7 @@ pub(crate) mod module { let env_str = format!("{key_str}={value_str}"); env_strings.push( - widestring::WideCString::from_os_str(&*std::ffi::OsString::from(env_str)) + widestring::WideCString::from_str(&env_str) .map_err(|err| err.to_pyexception(vm))?, ); } @@ -505,14 +501,12 @@ pub(crate) mod module { vm.new_runtime_error("exec not supported for isolated subinterpreters".to_owned()) ); } - let make_widestring = - |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm)); let path = path.to_wide_cstring(vm)?; - let argv = vm.extract_elements_with(argv.as_ref(), |obj| { let arg = PyStrRef::try_from_object(vm, obj)?; - make_widestring(arg.expect_str()) + widestring::WideCString::from_str(arg.expect_str()) + .map_err(|err| err.to_pyexception(vm)) })?; let first = argv @@ -540,14 +534,12 @@ pub(crate) mod module { vm.new_runtime_error("exec not supported for isolated subinterpreters".to_owned()) ); } - let make_widestring = - |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm)); let path = path.to_wide_cstring(vm)?; - let argv = vm.extract_elements_with(argv.as_ref(), |obj| { let arg = PyStrRef::try_from_object(vm, obj)?; - make_widestring(arg.expect_str()) + widestring::WideCString::from_str(arg.expect_str()) + .map_err(|err| err.to_pyexception(vm)) })?; let first = argv @@ -572,6 +564,7 @@ pub(crate) mod module { cold_path(); return Err(exceptions::nul_char_error(vm)); } + // Validate: empty key or '=' in key after position 0 // (search from index 1 because on Windows starting '=' is allowed // for defining hidden environment variables) @@ -580,7 +573,9 @@ pub(crate) mod module { } let env_str = format!("{key_str}={value_str}"); - env_strings.push(make_widestring(&env_str)?); + // SAFETY: Interior NULs are rejected above. WideCString appends the trailing NUL. + let env_str = unsafe { widestring::WideCString::from_str_unchecked(&env_str) }; + env_strings.push(env_str); } let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect();