From 455f4a615c124f68f5e121f9a90edaff5ac9f8ed Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Wed, 8 Jul 2026 21:47:59 -0400 Subject: [PATCH] ffi: No interior NULs (part 1) Interior NULs is a security hazard for C-style strings. A NUL byte truncates a string which can lead the caller and callee to see two different strings. It can cause path traversal attacks where a path in Python looks complete but it is interpreted differently through FFI. RustPython needs to handle this for some of its C-API as well as raw libc or Windows calls. Both Rust's standard library as well as Rustix handle interior NULs for us with CStrings, so this mostly affects a handful of Windows functions or areas where we have raw bytes that weren't checked by CString. Finally, this PR is non-exhaustive. I will have to rely heavily on CodeRabbit to help lint it to ensure that interior NUL checks are only introduced for FFI and not outside of it. Most of RustPython seems to handle interior NULs already due to CString as well as WideCString. **Sources:** * https://owasp.org/www-community/attacks/Embedding_Null_Code * python/cpython#11656 --- crates/host_env/src/ctypes.rs | 14 +- crates/host_env/src/fileutils.rs | 29 +- crates/host_env/src/nt.rs | 402 +++++++++-------------- crates/host_env/src/overlapped.rs | 17 +- crates/host_env/src/posix_windows.rs | 2 +- crates/host_env/src/winapi.rs | 6 + crates/host_env/src/windows.rs | 62 ++-- crates/host_env/src/winreg.rs | 41 +-- crates/host_env/src/wmi.rs | 4 +- crates/stdlib/src/overlapped.rs | 11 +- crates/vm/src/exceptions.rs | 9 +- crates/vm/src/stdlib/_codecs.rs | 13 +- crates/vm/src/stdlib/_ctypes/base.rs | 5 +- crates/vm/src/stdlib/_ctypes/function.rs | 2 +- crates/vm/src/stdlib/_io.rs | 5 +- crates/vm/src/stdlib/_winapi.rs | 153 +++++---- crates/vm/src/stdlib/nt.rs | 75 ++--- crates/vm/src/stdlib/os.rs | 16 +- crates/vm/src/stdlib/winreg.rs | 113 ++++--- crates/vm/src/stdlib/winsound.rs | 41 +-- 20 files changed, 495 insertions(+), 525 deletions(-) diff --git a/crates/host_env/src/ctypes.rs b/crates/host_env/src/ctypes.rs index a038e7a6d49..97a5fe5df55 100644 --- a/crates/host_env/src/ctypes.rs +++ b/crates/host_env/src/ctypes.rs @@ -33,8 +33,7 @@ use libloading::Library; use libloading::os::unix::Library as UnixLibrary; #[cfg(any(unix, windows))] use parking_lot::{Mutex, RwLock}; -use rustpython_wtf8::Wtf8; -use rustpython_wtf8::Wtf8Buf; +use rustpython_wtf8::{Wtf8, Wtf8Buf}; #[cfg(any(unix, windows))] use std::{collections::HashMap, ffi::OsStr, sync::OnceLock}; use widestring::WideCStr; @@ -1091,10 +1090,15 @@ pub fn utf16z_bytes(s: &Wtf8) -> Vec { .collect() } +/// Return a NUL terminated copy of `bytes`. +/// +/// The input may contain interior NULs. pub fn null_terminated_bytes(bytes: &[u8]) -> Vec { - let mut buffer = bytes.to_vec(); - buffer.push(0); - buffer + if bytes.last() == Some(&0) { + bytes.to_vec() + } else { + bytes.iter().copied().chain(Some(0)).collect() + } } pub fn decode_type_code(type_code: &str, bytes: &[u8]) -> DecodedValue { diff --git a/crates/host_env/src/fileutils.rs b/crates/host_env/src/fileutils.rs index a8e56bb1c0b..9d7e4430681 100644 --- a/crates/host_env/src/fileutils.rs +++ b/crates/host_env/src/fileutils.rs @@ -17,10 +17,10 @@ pub fn fstat(fd: crate::crt_fd::Borrowed<'_>) -> std::io::Result { #[cfg(windows)] pub mod windows { use crate::crt_fd; - use crate::windows::ToWideString; use libc::{S_IFCHR, S_IFDIR, S_IFMT}; use std::ffi::OsStr; use std::os::windows::io::AsRawHandle; + use std::path::Path; use std::sync::OnceLock; use windows_sys::Win32::Foundation::{ ERROR_INVALID_HANDLE, ERROR_NOT_SUPPORTED, FILETIME, FreeLibrary, SetLastError, @@ -67,21 +67,15 @@ pub mod windows { impl StatStruct { // update_st_mode_from_path in cpython pub fn update_st_mode_from_path(&mut self, path: &OsStr, attr: u32) { - if attr & FILE_ATTRIBUTE_DIRECTORY == 0 { - let file_extension = path - .to_wide() - .split(|&c| c == '.' as u16) - .next_back() - .and_then(|s| String::from_utf16(s).ok()); - - if let Some(file_extension) = file_extension - && (file_extension.eq_ignore_ascii_case("exe") - || file_extension.eq_ignore_ascii_case("bat") - || file_extension.eq_ignore_ascii_case("cmd") - || file_extension.eq_ignore_ascii_case("com")) - { - self.st_mode |= 0o111; - } + if attr & FILE_ATTRIBUTE_DIRECTORY == 0 + && let Some(file_extension) = + Path::new(path).extension().and_then(|ext| ext.to_str()) + && (file_extension.eq_ignore_ascii_case("exe") + || file_extension.eq_ignore_ascii_case("bat") + || file_extension.eq_ignore_ascii_case("cmd") + || file_extension.eq_ignore_ascii_case("com")) + { + self.st_mode |= 0o111; } } } @@ -288,7 +282,7 @@ pub mod windows { // _Py_GetFileInformationByName in cpython pub fn get_file_information_by_name( - file_name: &OsStr, + file_name: &widestring::WideCStr, file_information_class: FILE_INFO_BY_NAME_CLASS, ) -> std::io::Result { static GET_FILE_INFORMATION_BY_NAME: OnceLock< @@ -329,7 +323,6 @@ pub mod windows { }) .ok_or_else(|| std::io::Error::from_raw_os_error(ERROR_NOT_SUPPORTED as _))?; - let file_name = file_name.to_wide_with_nul(); let file_info_buffer_size = core::mem::size_of::() as u32; let mut file_info_buffer = core::mem::MaybeUninit::::uninit(); unsafe { diff --git a/crates/host_env/src/nt.rs b/crates/host_env/src/nt.rs index 7e0591600b1..8a571939a16 100644 --- a/crates/host_env/src/nt.rs +++ b/crates/host_env/src/nt.rs @@ -5,7 +5,7 @@ // cspell:ignore hchmod use std::{ - ffi::{OsStr, OsString}, + ffi::OsString, io, os::windows::{ffi::OsStringExt, io::AsRawHandle}, path::Path, @@ -19,27 +19,61 @@ use crate::{ StatStruct, windows::{FILE_INFO_BY_NAME_CLASS, get_file_information_by_name, stat_basic_info_to_stat}, }, - windows::{CheckWin32Bool, CheckWin32Handle, CheckWin32Sentinel, HandleToOwned, ToWideString}, + windows::{CheckWin32Bool, CheckWin32Handle, CheckWin32Sentinel, HandleToOwned}, }; use libc::intptr_t; +use widestring::WideCString; use windows_sys::{ Win32::{ Foundation::{ - CloseHandle, ERROR_INVALID_HANDLE, GetLastError, HANDLE, INVALID_HANDLE_VALUE, MAX_PATH, + CloseHandle, ERROR_ACCESS_DENIED, ERROR_BAD_NET_NAME, ERROR_BAD_NETPATH, + ERROR_BAD_PATHNAME, ERROR_CANT_ACCESS_FILE, ERROR_DIRECTORY, ERROR_FILE_NOT_FOUND, + ERROR_FILENAME_EXCED_RANGE, ERROR_INSUFFICIENT_BUFFER, ERROR_INVALID_FUNCTION, + ERROR_INVALID_HANDLE, ERROR_INVALID_NAME, ERROR_INVALID_PARAMETER, ERROR_MORE_DATA, + ERROR_NOT_READY, ERROR_NOT_SUPPORTED, ERROR_PATH_NOT_FOUND, ERROR_SHARING_VIOLATION, + GENERIC_READ, GENERIC_WRITE, GetHandleInformation, GetLastError, HANDLE, + HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, MAX_PATH, SetHandleInformation, }, Globalization::{CP_UTF8, MultiByteToWideChar, WideCharToMultiByte}, Storage::FileSystem::{ - CreateFileW, FILE_BASIC_INFO, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, - FILE_READ_ATTRIBUTES, FILE_TYPE_UNKNOWN, FileBasicInfo, FindClose, FindFirstFileW, - GetFileAttributesW, GetFileInformationByHandleEx, GetFileType, GetFullPathNameW, - INVALID_FILE_ATTRIBUTES, OPEN_EXISTING, SetFileAttributesW, SetFileInformationByHandle, - WIN32_FIND_DATAW, + BY_HANDLE_FILE_INFORMATION, CreateFileW, CreateSymbolicLinkW, DeleteFileW, + FILE_ATTRIBUTE_TAG_INFO, FILE_BASIC_INFO, FILE_DEVICE_CD_ROM, FILE_DEVICE_DISK, + FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, FILE_ID_INFO, + FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + FILE_TYPE_CHAR, FILE_TYPE_DISK, FILE_TYPE_PIPE, FILE_TYPE_UNKNOWN, + FILE_WRITE_ATTRIBUTES, FileAttributeTagInfo as FileAttributeTagInfoClass, + FileBasicInfo, FileIdInfo, FindClose, FindFirstFileW, GetDiskFreeSpaceExW, + GetDriveTypeW, GetFileAttributesExW, GetFileAttributesW, GetFileInformationByHandle, + GetFileInformationByHandleEx, GetFileType, GetFullPathNameW, GetLogicalDriveStringsW, + GetVolumePathNameW, GetVolumePathNamesForVolumeNameW, INVALID_FILE_ATTRIBUTES, + OPEN_EXISTING, RemoveDirectoryW, SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE, + SYMBOLIC_LINK_FLAG_DIRECTORY, SetFileAttributesW, SetFileInformationByHandle, + WIN32_FILE_ATTRIBUTE_DATA, WIN32_FIND_DATAW, + }, + System::{ + Console, + IO::DeviceIoControl, + Ioctl::{ + FILE_DEVICE_VIRTUAL_DISK, FSCTL_GET_REPARSE_POINT, + FSCTL_QUERY_PERSISTENT_VOLUME_STATE, + }, + SystemServices::IO_REPARSE_TAG_MOUNT_POINT, + Threading, + WindowsProgramming::{DRIVE_FIXED, GetUserNameW}, }, - System::{Console, Threading}, }, w, }; +pub use windows_sys::Win32::Storage::FileSystem::{ + FILE_ATTRIBUTE_ARCHIVE, FILE_ATTRIBUTE_COMPRESSED, FILE_ATTRIBUTE_DEVICE, + FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_ENCRYPTED, FILE_ATTRIBUTE_HIDDEN, + FILE_ATTRIBUTE_INTEGRITY_STREAM, FILE_ATTRIBUTE_NO_SCRUB_DATA, FILE_ATTRIBUTE_NORMAL, + FILE_ATTRIBUTE_NOT_CONTENT_INDEXED, FILE_ATTRIBUTE_OFFLINE, FILE_ATTRIBUTE_READONLY, + FILE_ATTRIBUTE_REPARSE_POINT, FILE_ATTRIBUTE_SPARSE_FILE, FILE_ATTRIBUTE_SYSTEM, + FILE_ATTRIBUTE_TEMPORARY, FILE_ATTRIBUTE_VIRTUAL, +}; + pub type Handle = HANDLE; pub const MAX_PATH_USIZE: usize = MAX_PATH as usize; pub const ERROR_INVALID_HANDLE_I32: i32 = ERROR_INVALID_HANDLE as i32; @@ -54,15 +88,6 @@ pub const LOAD_LIBRARY_SEARCH_SYSTEM32: u32 = pub const LOAD_LIBRARY_SEARCH_USER_DIRS: u32 = windows_sys::Win32::System::LibraryLoader::LOAD_LIBRARY_SEARCH_USER_DIRS; -pub use windows_sys::Win32::Storage::FileSystem::{ - FILE_ATTRIBUTE_ARCHIVE, FILE_ATTRIBUTE_COMPRESSED, FILE_ATTRIBUTE_DEVICE, - FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_ENCRYPTED, FILE_ATTRIBUTE_HIDDEN, - FILE_ATTRIBUTE_INTEGRITY_STREAM, FILE_ATTRIBUTE_NO_SCRUB_DATA, FILE_ATTRIBUTE_NORMAL, - FILE_ATTRIBUTE_NOT_CONTENT_INDEXED, FILE_ATTRIBUTE_OFFLINE, FILE_ATTRIBUTE_READONLY, - FILE_ATTRIBUTE_REPARSE_POINT, FILE_ATTRIBUTE_SPARSE_FILE, FILE_ATTRIBUTE_SYSTEM, - FILE_ATTRIBUTE_TEMPORARY, FILE_ATTRIBUTE_VIRTUAL, -}; - #[cfg(target_env = "msvc")] unsafe extern "C" { fn _cwait(termstat: *mut i32, procHandle: intptr_t, action: i32) -> intptr_t; @@ -100,13 +125,13 @@ struct FileAttributeTagInfo { reparse_tag: u32, } -fn win32_large_integer_to_time(li: i64) -> (libc::time_t, i32) { +const fn win32_large_integer_to_time(li: i64) -> (libc::time_t, i32) { let nsec = ((li % 10_000_000) * 100) as i32; let sec = (li / 10_000_000 - crate::fileutils::windows::SECS_BETWEEN_EPOCHS) as libc::time_t; (sec, nsec) } -fn win32_filetime_to_time(ft_low: u32, ft_high: u32) -> (libc::time_t, i32) { +const fn win32_filetime_to_time(ft_low: u32, ft_high: u32) -> (libc::time_t, i32) { let ticks = ((ft_high as i64) << 32) | (ft_low as i64); let nsec = ((ticks % 10_000_000) * 100) as i32; let sec = (ticks / 10_000_000 - crate::fileutils::windows::SECS_BETWEEN_EPOCHS) as libc::time_t; @@ -114,15 +139,11 @@ fn win32_filetime_to_time(ft_low: u32, ft_high: u32) -> (libc::time_t, i32) { } fn win32_attribute_data_to_stat( - info: &windows_sys::Win32::Storage::FileSystem::BY_HANDLE_FILE_INFORMATION, + info: &BY_HANDLE_FILE_INFORMATION, reparse_tag: u32, - basic_info: Option<&windows_sys::Win32::Storage::FileSystem::FILE_BASIC_INFO>, - id_info: Option<&windows_sys::Win32::Storage::FileSystem::FILE_ID_INFO>, + basic_info: Option<&FILE_BASIC_INFO>, + id_info: Option<&FILE_ID_INFO>, ) -> StatStruct { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_READONLY, FILE_ATTRIBUTE_REPARSE_POINT, - }; - let mut st_mode: u16 = 0; if info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0 { st_mode |= S_IFDIR_MODE | 0o111; @@ -221,37 +242,28 @@ pub enum ReadConsoleError { } pub fn access(path: &Path, mode: u8) -> bool { - let wide = path.as_os_str().to_wide_with_nul(); + let Ok(wide) = WideCString::from_os_str(path.as_os_str()) else { + return false; + }; let attr = unsafe { GetFileAttributesW(wide.as_ptr()) }; attr != INVALID_FILE_ATTRIBUTES && (mode & 2 == 0 || attr & FILE_ATTRIBUTE_READONLY == 0 - || attr & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_DIRECTORY != 0) + || attr & FILE_ATTRIBUTE_DIRECTORY != 0) } -pub fn remove(path: &Path) -> io::Result<()> { - use windows_sys::Win32::Storage::FileSystem::{ - DeleteFileW, RemoveDirectoryW, WIN32_FIND_DATAW, - }; - use windows_sys::Win32::System::SystemServices::{ - IO_REPARSE_TAG_MOUNT_POINT, IO_REPARSE_TAG_SYMLINK, - }; - - let wide_path = path.as_os_str().to_wide_with_nul(); - let attrs = unsafe { GetFileAttributesW(wide_path.as_ptr()) }; +pub fn remove(path: &widestring::WideCStr) -> io::Result<()> { + let attrs = unsafe { GetFileAttributesW(path.as_ptr()) }; let mut is_directory = false; let mut is_link = false; if attrs != INVALID_FILE_ATTRIBUTES { - is_directory = - (attrs & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_DIRECTORY) != 0; + is_directory = (attrs & FILE_ATTRIBUTE_DIRECTORY) != 0; - if is_directory - && (attrs & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT) != 0 - { + if is_directory && (attrs & FILE_ATTRIBUTE_REPARSE_POINT) != 0 { let mut find_data: WIN32_FIND_DATAW = unsafe { core::mem::zeroed() }; - let handle = unsafe { FindFirstFileW(wide_path.as_ptr(), &mut find_data) }; + let handle = unsafe { FindFirstFileW(path.as_ptr(), &mut find_data) }; if handle != INVALID_HANDLE_VALUE { is_link = find_data.dwReserved0 == IO_REPARSE_TAG_SYMLINK || find_data.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT; @@ -261,9 +273,9 @@ pub fn remove(path: &Path) -> io::Result<()> { } if is_directory && is_link { - unsafe { RemoveDirectoryW(wide_path.as_ptr()) } + unsafe { RemoveDirectoryW(path.as_ptr()) } } else { - unsafe { DeleteFileW(wide_path.as_ptr()) } + unsafe { DeleteFileW(path.as_ptr()) } } .check_win32_bool() } @@ -282,12 +294,6 @@ pub fn symlink( dst_wide: &widestring::WideCStr, target_is_directory: bool, ) -> io::Result<()> { - use windows_sys::Win32::Storage::FileSystem::WIN32_FILE_ATTRIBUTE_DATA; - use windows_sys::Win32::Storage::FileSystem::{ - CreateSymbolicLinkW, FILE_ATTRIBUTE_DIRECTORY, GetFileAttributesExW, - SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE, SYMBOLIC_LINK_FLAG_DIRECTORY, - }; - static HAS_UNPRIVILEGED_FLAG: AtomicBool = AtomicBool::new(true); fn check_dir(src: &Path, dst: &Path) -> bool { @@ -327,15 +333,11 @@ pub fn symlink( let mut result = unsafe { CreateSymbolicLinkW(dst_wide.as_ptr(), src_wide.as_ptr(), flags) }; if !result && HAS_UNPRIVILEGED_FLAG.load(Ordering::Relaxed) - && unsafe { windows_sys::Win32::Foundation::GetLastError() } - == windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER + && unsafe { GetLastError() } == ERROR_INVALID_PARAMETER { let flags = flags & !SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; result = unsafe { CreateSymbolicLinkW(dst_wide.as_ptr(), src_wide.as_ptr(), flags) }; - if result - || unsafe { windows_sys::Win32::Foundation::GetLastError() } - != windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER - { + if result || unsafe { GetLastError() } != ERROR_INVALID_PARAMETER { HAS_UNPRIVILEGED_FLAG.store(false, Ordering::Relaxed); } } @@ -383,23 +385,17 @@ pub fn fchmod(fd: i32, mode: u32, write_bit: u32) -> io::Result<()> { win32_hchmod(handle.as_raw_handle() as HANDLE, mode, write_bit) } -pub fn win32_lchmod(path: &OsStr, mode: u32, write_bit: u32) -> io::Result<()> { - let wide = path.to_wide_with_nul(); - let attr = unsafe { GetFileAttributesW(wide.as_ptr()) }.check_ne(INVALID_FILE_ATTRIBUTES)?; +pub fn win32_lchmod(path: &widestring::WideCStr, mode: u32, write_bit: u32) -> io::Result<()> { + let attr = unsafe { GetFileAttributesW(path.as_ptr()) }.check_ne(INVALID_FILE_ATTRIBUTES)?; let new_attr = if mode & write_bit != 0 { attr & !FILE_ATTRIBUTE_READONLY } else { attr | FILE_ATTRIBUTE_READONLY }; - unsafe { SetFileAttributesW(wide.as_ptr(), new_attr) }.check_win32_bool() + unsafe { SetFileAttributesW(path.as_ptr(), new_attr) }.check_win32_bool() } pub fn chmod_follow(path: &widestring::WideCStr, mode: u32, write_bit: u32) -> io::Result<()> { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_FLAG_BACKUP_SEMANTICS, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, - FILE_SHARE_WRITE, FILE_WRITE_ATTRIBUTES, OPEN_EXISTING, - }; - let handle = unsafe { CreateFileW( path.as_ptr(), @@ -416,11 +412,10 @@ pub fn chmod_follow(path: &widestring::WideCStr, mode: u32, write_bit: u32) -> i win32_hchmod(handle.as_raw_handle() as HANDLE, mode, write_bit) } -pub fn find_first_file_name(path: &Path) -> io::Result { - let wide_path = path.as_os_str().to_wide_with_nul(); +pub fn find_first_file_name(path: &widestring::WideCStr) -> io::Result { let mut find_data: WIN32_FIND_DATAW = unsafe { core::mem::zeroed() }; - let handle = unsafe { FindFirstFileW(wide_path.as_ptr(), &mut find_data) }.check_valid()?; + let handle = unsafe { FindFirstFileW(path.as_ptr(), &mut find_data) }.check_valid()?; unsafe { FindClose(handle) }; let len = find_data @@ -431,14 +426,7 @@ pub fn find_first_file_name(path: &Path) -> io::Result { Ok(OsString::from_wide(&find_data.cFileName[..len])) } -pub fn path_isdevdrive(path: &Path) -> io::Result { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_SHARE_READ, FILE_SHARE_WRITE, GetDriveTypeW, GetVolumePathNameW, - }; - use windows_sys::Win32::System::IO::DeviceIoControl; - use windows_sys::Win32::System::Ioctl::FSCTL_QUERY_PERSISTENT_VOLUME_STATE; - use windows_sys::Win32::System::WindowsProgramming::DRIVE_FIXED; - +pub fn path_isdevdrive(path: &widestring::WideCStr) -> io::Result { const PERSISTENT_VOLUME_STATE_DEV_VOLUME: u32 = 0x0000_2000; #[repr(C)] @@ -449,9 +437,8 @@ pub fn path_isdevdrive(path: &Path) -> io::Result { reserved: u32, } - let wide_path = path.as_os_str().to_wide_with_nul(); let mut volume = [0u16; MAX_PATH as usize]; - unsafe { GetVolumePathNameW(wide_path.as_ptr(), volume.as_mut_ptr(), volume.len() as _) } + unsafe { GetVolumePathNameW(path.as_ptr(), volume.as_mut_ptr(), volume.len() as _) } .check_win32_bool()?; if unsafe { GetDriveTypeW(volume.as_ptr()) } != DRIVE_FIXED { return Ok(false); @@ -503,38 +490,30 @@ pub fn path_isdevdrive(path: &Path) -> io::Result { Ok((volume_state.volume_flags & PERSISTENT_VOLUME_STATE_DEV_VOLUME) != 0) } -pub fn is_reparse_tag_name_surrogate(tag: u32) -> bool { +pub const fn is_reparse_tag_name_surrogate(tag: u32) -> bool { (tag & 0x20000000) != 0 } -pub fn file_info_error_is_trustworthy(error: u32) -> bool { - use windows_sys::Win32::Foundation; +pub const fn file_info_error_is_trustworthy(error: u32) -> bool { matches!( error, - Foundation::ERROR_FILE_NOT_FOUND - | Foundation::ERROR_PATH_NOT_FOUND - | Foundation::ERROR_NOT_READY - | Foundation::ERROR_BAD_NET_NAME - | Foundation::ERROR_BAD_NETPATH - | Foundation::ERROR_BAD_PATHNAME - | Foundation::ERROR_INVALID_NAME - | Foundation::ERROR_FILENAME_EXCED_RANGE + ERROR_FILE_NOT_FOUND + | ERROR_PATH_NOT_FOUND + | ERROR_NOT_READY + | ERROR_BAD_NET_NAME + | ERROR_BAD_NETPATH + | ERROR_BAD_PATHNAME + | ERROR_INVALID_NAME + | ERROR_FILENAME_EXCED_RANGE ) } -pub fn test_info( +pub const fn test_info( attributes: u32, reparse_tag: u32, disk_device: bool, tested_type: TestType, ) -> bool { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_REPARSE_POINT, - }; - use windows_sys::Win32::System::SystemServices::{ - IO_REPARSE_TAG_MOUNT_POINT, IO_REPARSE_TAG_SYMLINK, - }; - match tested_type { TestType::RegularFile => { disk_device && attributes != 0 && (attributes & FILE_ATTRIBUTE_DIRECTORY) == 0 @@ -561,10 +540,6 @@ pub fn test_info( } pub fn test_file_type_by_handle(handle: HANDLE, tested_type: TestType, disk_only: bool) -> bool { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_ATTRIBUTE_TAG_INFO, FILE_TYPE_DISK, FileAttributeTagInfo as FileAttributeTagInfoClass, - }; - let disk_device = unsafe { GetFileType(handle) } == FILE_TYPE_DISK; if disk_only && !disk_device { return false; @@ -607,19 +582,11 @@ pub fn test_file_type_by_handle(handle: HANDLE, tested_type: TestType, disk_only } fn win32_xstat_attributes_from_dir( - path: &OsStr, -) -> io::Result<( - windows_sys::Win32::Storage::FileSystem::BY_HANDLE_FILE_INFORMATION, - u32, -)> { - use windows_sys::Win32::Storage::FileSystem::{ - BY_HANDLE_FILE_INFORMATION, FILE_ATTRIBUTE_REPARSE_POINT, - }; - - let wide: Vec = path.to_wide_with_nul(); + path: &widestring::WideCStr, +) -> io::Result<(BY_HANDLE_FILE_INFORMATION, u32)> { let mut find_data: WIN32_FIND_DATAW = unsafe { core::mem::zeroed() }; - let handle = unsafe { FindFirstFileW(wide.as_ptr(), &mut find_data) }.check_valid()?; + let handle = unsafe { FindFirstFileW(path.as_ptr(), &mut find_data) }.check_valid()?; unsafe { FindClose(handle) }; let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { core::mem::zeroed() }; @@ -639,22 +606,7 @@ fn win32_xstat_attributes_from_dir( Ok((info, reparse_tag)) } -fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result { - use windows_sys::Win32::{ - Foundation::{ - ERROR_ACCESS_DENIED, ERROR_CANT_ACCESS_FILE, ERROR_INVALID_FUNCTION, - ERROR_INVALID_PARAMETER, ERROR_NOT_SUPPORTED, ERROR_SHARING_VIOLATION, GENERIC_READ, - }, - Storage::FileSystem::{ - BY_HANDLE_FILE_INFORMATION, FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_NORMAL, - FILE_ATTRIBUTE_REPARSE_POINT, FILE_BASIC_INFO, FILE_ID_INFO, FILE_SHARE_READ, - FILE_SHARE_WRITE, FILE_TYPE_CHAR, FILE_TYPE_PIPE, - FileAttributeTagInfo as FileAttributeTagInfoClass, FileBasicInfo, FileIdInfo, - GetFileAttributesW, GetFileInformationByHandle, - }, - }; - - let wide: Vec = path.to_wide_with_nul(); +fn win32_xstat_slow_impl(path: &widestring::WideCStr, traverse: bool) -> io::Result { let access = FILE_READ_ATTRIBUTES; let mut flags = FILE_FLAG_BACKUP_SEMANTICS; if !traverse { @@ -663,7 +615,7 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result let mut h_file = unsafe { CreateFileW( - wide.as_ptr(), + path.as_ptr(), access, 0, core::ptr::null(), @@ -694,7 +646,7 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result ERROR_INVALID_PARAMETER => { h_file = unsafe { CreateFileW( - wide.as_ptr(), + path.as_ptr(), access | GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, core::ptr::null(), @@ -711,7 +663,7 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result is_unhandled_tag = true; h_file = unsafe { CreateFileW( - wide.as_ptr(), + path.as_ptr(), access, 0, core::ptr::null(), @@ -731,14 +683,14 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result let result = (|| -> io::Result { if h_file != INVALID_HANDLE_VALUE { let file_type = unsafe { GetFileType(h_file) }; - if file_type != windows_sys::Win32::Storage::FileSystem::FILE_TYPE_DISK { + if file_type != FILE_TYPE_DISK { if file_type == FILE_TYPE_UNKNOWN { let err = io::Error::last_os_error(); if err.raw_os_error().unwrap_or(0) != 0 { return Err(err); } } - let file_attributes = unsafe { GetFileAttributesW(wide.as_ptr()) }; + let file_attributes = unsafe { GetFileAttributesW(path.as_ptr()) }; let mut st_mode = 0; if file_attributes != INVALID_FILE_ATTRIBUTES && file_attributes & FILE_ATTRIBUTE_DIRECTORY != 0 @@ -831,12 +783,12 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result }, if has_id_info { Some(&id_info) } else { None }, ); - result.update_st_mode_from_path(path, file_info.dwFileAttributes); + result.update_st_mode_from_path(&path.to_os_string(), file_info.dwFileAttributes); Ok(result) } else { let mut result = win32_attribute_data_to_stat(&file_info, tag_info.reparse_tag, None, None); - result.update_st_mode_from_path(path, file_info.dwFileAttributes); + result.update_st_mode_from_path(&path.to_os_string(), file_info.dwFileAttributes); Ok(result) } })(); @@ -847,9 +799,7 @@ fn win32_xstat_slow_impl(path: &OsStr, traverse: bool) -> io::Result result } -pub fn win32_xstat(path: &OsStr, traverse: bool) -> io::Result { - use windows_sys::Win32::{Foundation, Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT}; - +pub fn win32_xstat(path: &widestring::WideCStr, traverse: bool) -> io::Result { match get_file_information_by_name(path, FILE_INFO_BY_NAME_CLASS::FileStatBasicByNameInfo) { Ok(stat_info) => { if (stat_info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT == 0) @@ -857,7 +807,7 @@ pub fn win32_xstat(path: &OsStr, traverse: bool) -> io::Result { { let mut result = stat_basic_info_to_stat(&stat_info); if result.st_ino != 0 || result.st_ino_high != 0 { - result.update_st_mode_from_path(path, stat_info.FileAttributes); + result.update_st_mode_from_path(&path.to_os_string(), stat_info.FileAttributes); result.st_ctime = result.st_birthtime; result.st_ctime_nsec = result.st_birthtime_nsec; return Ok(result); @@ -868,10 +818,10 @@ pub fn win32_xstat(path: &OsStr, traverse: bool) -> io::Result { if let Some(errno) = err.raw_os_error() && matches!( errno as u32, - Foundation::ERROR_FILE_NOT_FOUND - | Foundation::ERROR_PATH_NOT_FOUND - | Foundation::ERROR_NOT_READY - | Foundation::ERROR_BAD_NET_NAME + ERROR_FILE_NOT_FOUND + | ERROR_PATH_NOT_FOUND + | ERROR_NOT_READY + | ERROR_BAD_NET_NAME ) { return Err(err); @@ -885,17 +835,12 @@ pub fn win32_xstat(path: &OsStr, traverse: bool) -> io::Result { Ok(result) } -pub fn test_file_type_by_name(path: &Path, tested_type: TestType) -> bool { - match get_file_information_by_name( - path.as_os_str(), - FILE_INFO_BY_NAME_CLASS::FileStatBasicByNameInfo, - ) { +pub fn test_file_type_by_name(path: &widestring::WideCStr, tested_type: TestType) -> bool { + match get_file_information_by_name(path, FILE_INFO_BY_NAME_CLASS::FileStatBasicByNameInfo) { Ok(info) => { let disk_device = matches!( info.DeviceType, - windows_sys::Win32::Storage::FileSystem::FILE_DEVICE_DISK - | windows_sys::Win32::System::Ioctl::FILE_DEVICE_VIRTUAL_DISK - | windows_sys::Win32::Storage::FileSystem::FILE_DEVICE_CD_ROM + FILE_DEVICE_DISK | FILE_DEVICE_VIRTUAL_DISK | FILE_DEVICE_CD_ROM ); let result = test_info( info.FileAttributes, @@ -905,9 +850,7 @@ pub fn test_file_type_by_name(path: &Path, tested_type: TestType) -> bool { ); if !result || !matches!(tested_type, TestType::RegularFile | TestType::Directory) - || (info.FileAttributes - & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT) - == 0 + || (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0 { return result; } @@ -925,10 +868,9 @@ pub fn test_file_type_by_name(path: &Path, tested_type: TestType) -> bool { if !matches!(tested_type, TestType::RegularFile | TestType::Directory) { flags |= FILE_FLAG_OPEN_REPARSE_POINT; } - let wide_path = path.as_os_str().to_wide_with_nul(); let handle = unsafe { CreateFileW( - wide_path.as_ptr(), + path.as_ptr(), FILE_READ_ATTRIBUTES, 0, core::ptr::null(), @@ -949,7 +891,7 @@ pub fn test_file_type_by_name(path: &Path, tested_type: TestType) -> bool { | windows_sys::Win32::Foundation::ERROR_CANT_ACCESS_FILE | windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER => { let stat = win32_xstat( - path.as_os_str(), + path, matches!(tested_type, TestType::RegularFile | TestType::Directory), ); if let Ok(st) = stat { @@ -968,15 +910,10 @@ pub fn test_file_type_by_name(path: &Path, tested_type: TestType) -> bool { false } -pub fn test_file_exists_by_name(path: &Path, follow_links: bool) -> bool { - match get_file_information_by_name( - path.as_os_str(), - FILE_INFO_BY_NAME_CLASS::FileStatBasicByNameInfo, - ) { +pub fn test_file_exists_by_name(path: &widestring::WideCStr, follow_links: bool) -> bool { + match get_file_information_by_name(path, FILE_INFO_BY_NAME_CLASS::FileStatBasicByNameInfo) { Ok(info) => { - if (info.FileAttributes - & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT) - == 0 + if (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0 || (!follow_links && is_reparse_tag_name_surrogate(info.ReparseTag)) { return true; @@ -991,14 +928,13 @@ pub fn test_file_exists_by_name(path: &Path, follow_links: bool) -> bool { } } - let wide_path = path.as_os_str().to_wide_with_nul(); let mut flags = FILE_FLAG_BACKUP_SEMANTICS; if !follow_links { flags |= FILE_FLAG_OPEN_REPARSE_POINT; } let handle = unsafe { CreateFileW( - wide_path.as_ptr(), + path.as_ptr(), FILE_READ_ATTRIBUTES, 0, core::ptr::null(), @@ -1020,7 +956,7 @@ pub fn test_file_exists_by_name(path: &Path, follow_links: bool) -> bool { } let handle = unsafe { CreateFileW( - wide_path.as_ptr(), + path.as_ptr(), FILE_READ_ATTRIBUTES, 0, core::ptr::null(), @@ -1036,11 +972,11 @@ pub fn test_file_exists_by_name(path: &Path, follow_links: bool) -> bool { } match unsafe { GetLastError() } { - windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED - | windows_sys::Win32::Foundation::ERROR_SHARING_VIOLATION - | windows_sys::Win32::Foundation::ERROR_CANT_ACCESS_FILE - | windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER => { - return win32_xstat(path.as_os_str(), follow_links).is_ok(); + ERROR_ACCESS_DENIED + | ERROR_SHARING_VIOLATION + | ERROR_CANT_ACCESS_FILE + | ERROR_INVALID_PARAMETER => { + return win32_xstat(path, follow_links).is_ok(); } _ => {} } @@ -1049,7 +985,9 @@ pub fn test_file_exists_by_name(path: &Path, follow_links: bool) -> bool { } pub fn path_exists_via_open(path: &Path, follow_links: bool) -> bool { - let wide_path = path.as_os_str().to_wide_with_nul(); + let Ok(wide_path) = WideCString::from_os_str(path.as_os_str()) else { + return false; + }; let mut flags = FILE_FLAG_BACKUP_SEMANTICS; if !follow_links { flags |= FILE_FLAG_OPEN_REPARSE_POINT; @@ -1260,21 +1198,10 @@ pub fn dup2(fd: i32, fd2: i32, inheritable: bool) -> io::Result { Ok(fd2) } -pub fn readlink(path: &Path) -> Result { - use windows_sys::Win32::Storage::FileSystem::{ - FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, FILE_SHARE_DELETE, - FILE_SHARE_READ, FILE_SHARE_WRITE, - }; - use windows_sys::Win32::System::IO::DeviceIoControl; - use windows_sys::Win32::System::Ioctl::FSCTL_GET_REPARSE_POINT; - use windows_sys::Win32::System::SystemServices::{ - IO_REPARSE_TAG_MOUNT_POINT, IO_REPARSE_TAG_SYMLINK, - }; - - let wide_path = path.as_os_str().to_wide_with_nul(); +pub fn readlink(path: &widestring::WideCStr) -> Result { let handle = unsafe { CreateFileW( - wide_path.as_ptr(), + path.as_ptr(), 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, core::ptr::null(), @@ -1369,13 +1296,12 @@ pub fn kill(pid: u32, sig: u32) -> io::Result<()> { } } -pub fn getfinalpathname(path: &Path) -> io::Result { +pub fn getfinalpathname(path: &widestring::WideCStr) -> io::Result { use windows_sys::Win32::Storage::FileSystem::{GetFinalPathNameByHandleW, VOLUME_NAME_DOS}; - let wide = path.as_os_str().to_wide_with_nul(); let handle = unsafe { CreateFileW( - wide.as_ptr(), + path.as_ptr(), 0, 0, core::ptr::null(), @@ -1409,12 +1335,11 @@ pub fn getfinalpathname(path: &Path) -> io::Result { result } -pub fn getfullpathname(path: &Path) -> io::Result { - let wide = path.as_os_str().to_wide_with_nul(); +pub fn getfullpathname(path: &widestring::WideCStr) -> io::Result { let mut buffer = vec![0u16; MAX_PATH as usize]; let mut ret = unsafe { windows_sys::Win32::Storage::FileSystem::GetFullPathNameW( - wide.as_ptr(), + path.as_ptr(), buffer.len() as u32, buffer.as_mut_ptr(), core::ptr::null_mut(), @@ -1425,7 +1350,7 @@ pub fn getfullpathname(path: &Path) -> io::Result { buffer.resize(ret as usize, 0); ret = unsafe { windows_sys::Win32::Storage::FileSystem::GetFullPathNameW( - wide.as_ptr(), + path.as_ptr(), buffer.len() as u32, buffer.as_mut_ptr(), core::ptr::null_mut(), @@ -1437,13 +1362,12 @@ pub fn getfullpathname(path: &Path) -> io::Result { Ok(widestring::WideCString::from_vec_truncate(buffer).to_os_string()) } -pub fn getvolumepathname(path: &Path) -> io::Result { - let wide = path.as_os_str().to_wide_with_nul(); - let buflen = core::cmp::max(wide.len(), MAX_PATH as usize); +pub fn getvolumepathname(path: &widestring::WideCStr) -> io::Result { + let buflen = core::cmp::max(path.len(), MAX_PATH as usize); let mut buffer = vec![0u16; buflen]; unsafe { windows_sys::Win32::Storage::FileSystem::GetVolumePathNameW( - wide.as_ptr(), + path.as_ptr(), buffer.as_mut_ptr(), buflen as u32, ) @@ -1452,23 +1376,21 @@ pub fn getvolumepathname(path: &Path) -> io::Result { Ok(widestring::WideCString::from_vec_truncate(buffer).to_os_string()) } -pub fn getdiskusage(path: &Path) -> io::Result<(u64, u64)> { - use windows_sys::Win32::Storage::FileSystem::GetDiskFreeSpaceExW; - - let wide = path.as_os_str().to_wide_with_nul(); +pub fn getdiskusage(path: &widestring::WideCStr) -> io::Result<(u64, u64)> { let mut free_to_me = 0u64; let mut total = 0u64; let mut free = 0u64; - let ok = unsafe { GetDiskFreeSpaceExW(wide.as_ptr(), &mut free_to_me, &mut total, &mut free) }; + let ok = unsafe { GetDiskFreeSpaceExW(path.as_ptr(), &mut free_to_me, &mut total, &mut free) }; if ok != 0 { return Ok((total, free)); } let err = io::Error::last_os_error(); - if err.raw_os_error() == Some(windows_sys::Win32::Foundation::ERROR_DIRECTORY as i32) - && let Some(parent) = path.parent() + if err.raw_os_error() == Some(ERROR_DIRECTORY as i32) + && let Some(parent) = Path::new(&path.to_os_string()).parent() { - let parent = widestring::WideCString::from_os_str(parent).unwrap(); + let parent = widestring::WideCString::from_os_str(parent) + .expect("interior NULs are impossible because parent was constructed from a WideCStr"); let ok = unsafe { GetDiskFreeSpaceExW(parent.as_ptr(), &mut free_to_me, &mut total, &mut free) }; if ok != 0 { @@ -1480,28 +1402,17 @@ pub fn getdiskusage(path: &Path) -> io::Result<(u64, u64)> { pub fn get_handle_inheritable(handle: intptr_t) -> io::Result { let mut flags = 0; - let ok = - unsafe { windows_sys::Win32::Foundation::GetHandleInformation(handle as _, &mut flags) }; + let ok = unsafe { GetHandleInformation(handle as _, &mut flags) }; if ok == 0 { Err(io::Error::last_os_error()) } else { - Ok(flags & windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT != 0) + Ok(flags & HANDLE_FLAG_INHERIT != 0) } } pub fn set_handle_inheritable(handle: intptr_t, inheritable: bool) -> io::Result<()> { - let flags = if inheritable { - windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT - } else { - 0 - }; - let ok = unsafe { - windows_sys::Win32::Foundation::SetHandleInformation( - handle as _, - windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT, - flags, - ) - }; + let flags = if inheritable { HANDLE_FLAG_INHERIT } else { 0 }; + let ok = unsafe { SetHandleInformation(handle as _, HANDLE_FLAG_INHERIT, flags) }; if ok == 0 { Err(io::Error::last_os_error()) } else { @@ -1512,9 +1423,7 @@ pub fn set_handle_inheritable(handle: intptr_t, inheritable: bool) -> io::Result pub fn getlogin() -> io::Result { let mut buffer = [0u16; 257]; let mut size = buffer.len() as u32; - let ok = unsafe { - windows_sys::Win32::System::WindowsProgramming::GetUserNameW(buffer.as_mut_ptr(), &mut size) - }; + let ok = unsafe { GetUserNameW(buffer.as_mut_ptr(), &mut size) }; if ok == 0 { return Err(io::Error::last_os_error()); } @@ -1526,19 +1435,12 @@ pub fn getlogin() -> io::Result { pub fn listdrives() -> io::Result> { let mut buffer = [0u16; 256]; - let len = unsafe { - windows_sys::Win32::Storage::FileSystem::GetLogicalDriveStringsW( - buffer.len() as u32, - buffer.as_mut_ptr(), - ) - }; + let len = unsafe { GetLogicalDriveStringsW(buffer.len() as u32, buffer.as_mut_ptr()) }; if len == 0 { return Err(io::Error::last_os_error()); } if len as usize >= buffer.len() { - return Err(io::Error::from_raw_os_error( - windows_sys::Win32::Foundation::ERROR_MORE_DATA as i32, - )); + return Err(io::Error::from_raw_os_error(ERROR_MORE_DATA as i32)); } Ok(buffer[..(len - 1) as usize] .split(|&c| c == 0) @@ -1586,15 +1488,14 @@ pub fn listvolumes() -> io::Result> { Ok(result) } -pub fn listmounts(volume: &Path) -> io::Result> { - let wide = volume.as_os_str().to_wide_with_nul(); +pub fn listmounts(volume: &widestring::WideCStr) -> io::Result> { let mut buflen: u32 = MAX_PATH + 1; let mut buffer = vec![0u16; buflen as usize]; loop { let ok = unsafe { - windows_sys::Win32::Storage::FileSystem::GetVolumePathNamesForVolumeNameW( - wide.as_ptr(), + GetVolumePathNamesForVolumeNameW( + volume.as_ptr(), buffer.as_mut_ptr(), buflen, &mut buflen, @@ -1604,7 +1505,7 @@ pub fn listmounts(volume: &Path) -> io::Result> { break; } let err = io::Error::last_os_error(); - if err.raw_os_error() == Some(windows_sys::Win32::Foundation::ERROR_MORE_DATA as i32) { + if err.raw_os_error() == Some(ERROR_MORE_DATA as i32) { buffer.resize(buflen as usize, 0); continue; } @@ -1679,6 +1580,7 @@ pub fn getppid() -> u32 { pub fn path_skip_root(path: &widestring::WideCStr) -> Option { let mut end: *const u16 = core::ptr::null(); + // SAFETY: `path` is a valid pointer to a nul terminated wide string without interior nuls. let hr = unsafe { windows_sys::Win32::UI::Shell::PathCchSkipRoot(path.as_ptr(), &mut end) }; if hr >= 0 { assert!(!end.is_null()); @@ -1697,19 +1599,17 @@ pub fn get_terminal_size_handle(h: HANDLE) -> io::Result<(usize, usize)> { let ret = unsafe { Console::GetConsoleScreenBufferInfo(h, csbi.as_mut_ptr()) }; if ret == 0 { let err = unsafe { GetLastError() }; - if err != windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED { + if err != ERROR_ACCESS_DENIED { return Err(io::Error::last_os_error()); } let conout = w!("CONOUT$"); let console_handle = unsafe { CreateFileW( conout, - windows_sys::Win32::Foundation::GENERIC_READ - | windows_sys::Win32::Foundation::GENERIC_WRITE, - windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ - | windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE, + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, core::ptr::null(), - windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING, + OPEN_EXISTING, 0, core::ptr::null_mut(), ) @@ -1964,8 +1864,7 @@ pub fn read_console_into( } let err = io::Error::last_os_error(); - if err.raw_os_error() == Some(windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER as i32) - { + if err.raw_os_error() == Some(ERROR_INSUFFICIENT_BUFFER as i32) { let needed = unsafe { WideCharToMultiByte( CP_UTF8, @@ -2119,11 +2018,6 @@ pub fn write_console_utf8(handle: HANDLE, data: &[u8], max_bytes: usize) -> io:: } pub fn open_console_path_fd(path: &widestring::WideCStr, writable: bool) -> io::Result { - use windows_sys::Win32::{ - Foundation::{GENERIC_READ, GENERIC_WRITE}, - Storage::FileSystem::{FILE_SHARE_READ, FILE_SHARE_WRITE}, - }; - let access = if writable { GENERIC_WRITE } else { diff --git a/crates/host_env/src/overlapped.rs b/crates/host_env/src/overlapped.rs index be5e75f585f..2547ab9f58b 100644 --- a/crates/host_env/src/overlapped.rs +++ b/crates/host_env/src/overlapped.rs @@ -15,7 +15,9 @@ use std::{ sync::{Mutex, OnceLock}, }; -use crate::windows::{CheckWin32Bool, CheckWin32Handle}; +use crate::windows::{CheckWin32Bool, CheckWin32Handle, ToWideString}; +use rustpython_wtf8::Wtf8; +use widestring::WideCStr; use windows_sys::Win32::{ Foundation::{CloseHandle, ERROR_IO_PENDING, ERROR_MORE_DATA, ERROR_SUCCESS, HANDLE}, Networking::WinSock::{AF_INET, AF_INET6, SOCKADDR, SOCKADDR_IN, SOCKADDR_IN6}, @@ -1020,7 +1022,10 @@ pub fn bind_local(socket: isize, family: i32) -> io::Result<()> { } } -pub fn parse_address_v4_wide(host_wide: &[u16], port: u16) -> io::Result<(Vec, i32)> { +pub fn parse_address_v4_wide( + host_wide: &widestring::WideCStr, + port: u16, +) -> io::Result<(Vec, i32)> { use windows_sys::Win32::Networking::WinSock::{WSAGetLastError, WSAStringToAddressW}; let mut addr: SOCKADDR_IN = unsafe { core::mem::zeroed() }; @@ -1028,6 +1033,7 @@ pub fn parse_address_v4_wide(host_wide: &[u16], port: u16) -> io::Result<(Vec() as i32; + // SAFETY: host_wide is nul capped and doesn't have interior nuls let ret = unsafe { WSAStringToAddressW( host_wide.as_ptr(), @@ -1056,7 +1062,7 @@ pub fn parse_address_v4_wide(host_wide: &[u16], port: u16) -> io::Result<(Vec io::Result<(Vec, i32)> { - let host_wide: Vec = host.encode_utf16().chain([0]).collect(); + let host_wide = Wtf8::new(host).to_wide_cstring()?; parse_address_v4_wide(&host_wide, port) } @@ -1066,12 +1072,12 @@ pub fn parse_address_v6( flowinfo: u32, scope_id: u32, ) -> io::Result<(Vec, i32)> { - let host_wide: Vec = host.encode_utf16().chain([0]).collect(); + let host_wide = Wtf8::new(host).to_wide_cstring()?; parse_address_v6_wide(&host_wide, port, flowinfo, scope_id) } pub fn parse_address_v6_wide( - host_wide: &[u16], + host_wide: &WideCStr, port: u16, flowinfo: u32, scope_id: u32, @@ -1083,6 +1089,7 @@ pub fn parse_address_v6_wide( let mut addr_len = core::mem::size_of::() as i32; + // SAFETY: host_wide is nul capped and doesn't have interior nuls let ret = unsafe { WSAStringToAddressW( host_wide.as_ptr(), diff --git a/crates/host_env/src/posix_windows.rs b/crates/host_env/src/posix_windows.rs index e78bd8f743f..61c20e9b229 100644 --- a/crates/host_env/src/posix_windows.rs +++ b/crates/host_env/src/posix_windows.rs @@ -78,7 +78,7 @@ fn rename_impl( .into_vec_with_nul(); // SAFETY: - // * from and to are NUL terminated wide strings + // * from and to are NUL terminated wide strings without interior nuls let success = unsafe { // Rust's [`std::fs::rename`] is more complicated than CPython's. Rust attempts to use modern APIs // where available, such as `FileRenameInfoEx`, which better map to POSIX. CPython simply diff --git a/crates/host_env/src/winapi.rs b/crates/host_env/src/winapi.rs index 19e18f32d3f..31783847311 100644 --- a/crates/host_env/src/winapi.rs +++ b/crates/host_env/src/winapi.rs @@ -1140,6 +1140,12 @@ pub fn lc_map_string_ex( src: &[u16], ) -> io::Result> { let src_len = src.len() as i32; + // SAFETY: + // * locale does not have interior NULs and ends with a NUL. This is guaranteed by + // WideCStr. + // * src CAN have interior NULs and DOES NOT need to end with a NUL. However, the length must be + // passed into LCMapStringEx. If the length is NOT passed in, Windows calculates the length + // and interior NULs are not allowed. let dest_size = unsafe { windows_sys::Win32::Globalization::LCMapStringEx( locale.as_ptr(), diff --git a/crates/host_env/src/windows.rs b/crates/host_env/src/windows.rs index 635f12f3f38..54710467c33 100644 --- a/crates/host_env/src/windows.rs +++ b/crates/host_env/src/windows.rs @@ -2,8 +2,9 @@ use rustpython_wtf8::Wtf8; use std::{ ffi::{OsStr, OsString}, io, - os::windows::ffi::{OsStrExt, OsStringExt}, + os::windows::ffi::OsStringExt, }; +use widestring::WideCString; use windows_sys::{ Win32::{ Foundation::{ @@ -397,54 +398,39 @@ pub fn multi_byte_to_wide( } } +/// [`OsStr`] to [`WideCString`] for Windows FFI. +/// +/// Prefer using this trait when encoding bytes to pass to Windows. Interior NULs are memory safe +/// but possibly a security hazard for FFI. +/// +/// https://github.com/python/cpython/issues/111656 pub trait ToWideString { - fn to_wide(&self) -> Vec; - fn to_wide_with_nul(&self) -> Vec; - fn to_wide_cstring(&self) -> widestring::WideCString { - widestring::WideCString::from_vec_truncate(self.to_wide()) - } + fn to_wide_cstring(&self) -> Result; } impl ToWideString for T where T: AsRef, { - fn to_wide(&self) -> Vec { - self.as_ref().encode_wide().collect() - } - fn to_wide_with_nul(&self) -> Vec { - self.as_ref().encode_wide().chain(Some(0)).collect() - } -} - -impl ToWideString for OsStr { - fn to_wide(&self) -> Vec { - self.encode_wide().collect() - } - fn to_wide_with_nul(&self) -> Vec { - self.encode_wide().chain(Some(0)).collect() + fn to_wide_cstring(&self) -> Result { + WideCString::from_os_str(self).map_err(|_| io::Error::other("embedded null character")) } } impl ToWideString for Wtf8 { - fn to_wide(&self) -> Vec { - self.encode_wide().collect() - } - fn to_wide_with_nul(&self) -> Vec { - self.encode_wide().chain(Some(0)).collect() - } -} - -pub trait FromWideString -where - Self: Sized, -{ - fn from_wides_until_nul(wide: &[u16]) -> Self; -} + fn to_wide_cstring(&self) -> Result { + // CPython's "test_invalid_cmd" test calls Popen with "pass#\0" as a command line. + // That's technically valid since it caps the string, but CString and WideCString differ + // in how they handle it. Rust's CString rejects any NULs whereas WideCString accepts a NUL + // only if it appears at the end of a buffer. + // + // 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")); + } -impl FromWideString for OsString { - fn from_wides_until_nul(wide: &[u16]) -> Self { - let len = wide.iter().take_while(|&&c| c != 0).count(); - Self::from_wide(&wide[..len]) + 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")) } } diff --git a/crates/host_env/src/winreg.rs b/crates/host_env/src/winreg.rs index 5324ac258e2..5a67e4e2b89 100644 --- a/crates/host_env/src/winreg.rs +++ b/crates/host_env/src/winreg.rs @@ -14,9 +14,7 @@ extern crate alloc; use alloc::string::FromUtf16Error; -use std::ffi::OsStr; -use crate::windows::ToWideString; use windows_sys::Win32::{ Foundation, Security::SECURITY_ATTRIBUTES, @@ -347,20 +345,11 @@ pub enum QueryStringError { pub fn query_default_value( hkey: Registry::HKEY, - sub_key: Option<&OsStr>, + sub_key: Option<&widestring::WideCStr>, ) -> Result { let child_key = if let Some(sub_key) = sub_key.filter(|s| !s.is_empty()) { - let wide_sub_key = sub_key.to_wide_cstring(); let mut out_key = core::ptr::null_mut(); - let res = unsafe { - open_key_ex( - hkey, - &wide_sub_key, - 0, - Registry::KEY_QUERY_VALUE, - &mut out_key, - ) - }; + let res = unsafe { open_key_ex(hkey, sub_key, 0, Registry::KEY_QUERY_VALUE, &mut out_key) }; if res != 0 { return Err(QueryStringError::Code(res)); } @@ -415,13 +404,15 @@ pub fn query_default_value( result } -pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Vec, u32), u32> { - let wide_name = value_name.to_wide_cstring(); +pub fn query_value_bytes( + hkey: Registry::HKEY, + wide_name: &widestring::WideCStr, +) -> Result<(Vec, u32), u32> { let mut buf_size: u32 = 0; let res = unsafe { query_value_ex( hkey, - Some(&wide_name), + Some(wide_name), core::ptr::null_mut(), core::ptr::null_mut(), &mut buf_size, @@ -441,7 +432,7 @@ pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Ve let res = unsafe { query_value_ex( hkey, - Some(&wide_name), + Some(wide_name), &mut typ, ret_buf.as_mut_ptr(), &mut ret_size, @@ -459,14 +450,18 @@ pub fn query_value_bytes(hkey: Registry::HKEY, value_name: &OsStr) -> Result<(Ve } } -pub fn set_default_value(hkey: Registry::HKEY, sub_key: &OsStr, typ: u32, value: &OsStr) -> u32 { +pub fn set_default_value( + hkey: Registry::HKEY, + sub_key: &widestring::WideCStr, + typ: u32, + wide_value: &widestring::WideStr, +) -> u32 { let child_key = if !sub_key.is_empty() { - let wide_sub_key = sub_key.to_wide_cstring(); let mut out_key = core::ptr::null_mut(); let res = unsafe { create_key_ex( hkey, - &wide_sub_key, + sub_key, 0, core::ptr::null_mut(), 0, @@ -485,7 +480,6 @@ pub fn set_default_value(hkey: Registry::HKEY, sub_key: &OsStr, typ: u32, value: }; let target_key = child_key.unwrap_or(hkey); - let wide_value = value.to_wide_with_nul(); let res = unsafe { set_value_ex( target_key, @@ -502,8 +496,9 @@ pub fn set_default_value(hkey: Registry::HKEY, sub_key: &OsStr, typ: u32, value: res } -pub fn expand_environment_strings(input: &OsStr) -> Result { - let wide_input = input.to_wide_with_nul(); +pub fn expand_environment_strings( + wide_input: &widestring::WideCStr, +) -> Result { let required_size = unsafe { Environment::ExpandEnvironmentStringsW(wide_input.as_ptr(), core::ptr::null_mut(), 0) }; diff --git a/crates/host_env/src/wmi.rs b/crates/host_env/src/wmi.rs index 2b46eebcbe5..74620048492 100644 --- a/crates/host_env/src/wmi.rs +++ b/crates/host_env/src/wmi.rs @@ -556,8 +556,8 @@ unsafe fn query_thread_impl(param: *mut c_void) -> u32 { pub fn exec_query(query_str: &str) -> Result { let query = WideCString::from_str(query_str) - .map_err(|_| ExecQueryError::Code(ERROR_INVALID_NAME))? - .into(); + .map(WideCString::into_vec_with_nul) + .map_err(|_| ExecQueryError::Code(ERROR_INVALID_NAME))?; let mut h_thread: HANDLE = null_mut(); let mut err: u32 = 0; diff --git a/crates/stdlib/src/overlapped.rs b/crates/stdlib/src/overlapped.rs index 86ac24e3a0f..153e723cc67 100644 --- a/crates/stdlib/src/overlapped.rs +++ b/crates/stdlib/src/overlapped.rs @@ -19,6 +19,7 @@ mod _overlapped { }; use rustpython_host_env::{ overlapped as host_overlapped, winapi as host_winapi, windows as host_windows, + windows::ToWideString, }; pub(crate) fn module_exec(vm: &VirtualMachine, module: &Py) -> PyResult<()> { @@ -210,7 +211,10 @@ mod _overlapped { // IPv4: (host, port) let host: PyStrRef = addr_obj[0].clone().try_into_value(vm)?; let port: u16 = addr_obj[1].clone().try_to_value(vm)?; - let host_wide: Vec = host.as_wtf8().encode_wide().chain([0]).collect(); + let host_wide = host + .as_wtf8() + .to_wide_cstring() + .map_err(|e| e.to_pyexception(vm))?; host_overlapped::parse_address_v4_wide(&host_wide, port) .map_err(|err| set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm)) } @@ -220,7 +224,10 @@ mod _overlapped { let port: u16 = addr_obj[1].clone().try_to_value(vm)?; let flowinfo: u32 = addr_obj[2].clone().try_to_value(vm)?; let scope_id: u32 = addr_obj[3].clone().try_to_value(vm)?; - let host_wide: Vec = host.as_wtf8().encode_wide().chain([0]).collect(); + let host_wide = host + .as_wtf8() + .to_wide_cstring() + .map_err(|e| e.to_pyexception(vm))?; host_overlapped::parse_address_v6_wide(&host_wide, port, flowinfo, scope_id) .map_err(|err| set_from_windows_err(err.raw_os_error().unwrap_or(0) as u32, vm)) } diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 0a1c2cb75ee..e5f296b0292 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -1251,7 +1251,14 @@ impl ToPyException for widestring::error::ContainsNul { #[cfg(windows)] impl ToPyException for widestring::error::MissingNulTerminator { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { - vm.new_value_error(self.to_string()) + nul_char_error(vm) + } +} + +#[cfg(windows)] +impl ToPyException for widestring::error::NulError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + nul_char_error(vm) } } diff --git a/crates/vm/src/stdlib/_codecs.rs b/crates/vm/src/stdlib/_codecs.rs index 497d62fcc81..22d15fd33dd 100644 --- a/crates/vm/src/stdlib/_codecs.rs +++ b/crates/vm/src/stdlib/_codecs.rs @@ -381,6 +381,7 @@ mod _codecs_windows { use crate::{PyResult, VirtualMachine}; use crate::{builtins::PyStrRef, builtins::PyUtf8StrRef, function::ArgBytesLike}; use rustpython_host_env::windows as host_windows; + use std::{ffi::OsStr, os::windows::ffi::OsStrExt}; #[derive(FromArgs)] struct MbcsEncodeArgs { @@ -392,8 +393,6 @@ mod _codecs_windows { #[pyfunction] fn mbcs_encode(args: MbcsEncodeArgs, vm: &VirtualMachine) -> PyResult<(Vec, usize)> { - use crate::host_env::windows::ToWideString; - let errors = args.errors.as_ref().map_or("strict", |s| s.as_str()); let s = match args.s.to_str() { Some(s) => s, @@ -411,7 +410,7 @@ mod _codecs_windows { } // Convert UTF-8 string to UTF-16 - let wide: Vec = std::ffi::OsStr::new(s).to_wide(); + let wide: Vec<_> = OsStr::new(s).encode_wide().collect(); // Get the required buffer size let (size, _) = host_windows::wide_char_to_multi_byte_len( @@ -516,8 +515,6 @@ mod _codecs_windows { #[pyfunction] fn oem_encode(args: OemEncodeArgs, vm: &VirtualMachine) -> PyResult<(Vec, usize)> { - use crate::host_env::windows::ToWideString; - let errors = args.errors.as_ref().map_or("strict", |s| s.as_str()); let s = match args.s.to_str() { Some(s) => s, @@ -535,7 +532,7 @@ mod _codecs_windows { } // Convert UTF-8 string to UTF-16 - let wide: Vec = std::ffi::OsStr::new(s).to_wide(); + let wide: Vec<_> = OsStr::new(s).encode_wide().collect(); // Get the required buffer size let (size, _) = host_windows::wide_char_to_multi_byte_len( @@ -879,8 +876,6 @@ mod _codecs_windows { args: CodePageEncodeArgs, vm: &VirtualMachine, ) -> PyResult<(Vec, usize)> { - use crate::host_env::windows::ToWideString; - if args.code_page < 0 { return Err(vm.new_value_error("invalid code page number")); } @@ -897,7 +892,7 @@ mod _codecs_windows { // Fast path: try encoding the whole string at once (only if no surrogates) if let Some(str_data) = args.s.to_str() { - let wide: Vec = std::ffi::OsStr::new(str_data).to_wide(); + let wide: Vec<_> = OsStr::new(str_data).encode_wide().collect(); if let Some(result) = try_encode_code_page_strict(code_page, &wide, vm)? { return Ok((result, char_len)); } diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index e86fdbc7a42..b422e411a63 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -19,7 +19,7 @@ use rustpython_common::lock::PyRwLock; use rustpython_common::wtf8::Wtf8; use rustpython_host_env::ctypes::{ CTypeLayout, char_array_assignment_bytes, char_array_field_value, wchar_array_field_value, - write_cow_bytes_at_offset, + wchar_null_terminated_bytes, write_cow_bytes_at_offset, }; // StgInfo - Storage information for ctypes types @@ -381,6 +381,7 @@ pub(super) static CDATA_BUFFER_METHODS: BufferMethods = BufferMethods { }; /// Ensure PyBytes data is null-terminated. Returns (kept_alive_obj, pointer). +/// /// The caller must keep the returned object alive to keep the pointer valid. pub(super) fn ensure_z_null_terminated( bytes: &PyBytes, @@ -394,7 +395,7 @@ pub(super) fn ensure_z_null_terminated( /// Convert str to null-terminated wchar_t buffer. Returns (PyBytes holder, pointer). pub(super) fn str_to_wchar_bytes(s: &Wtf8, vm: &VirtualMachine) -> (PyObjectRef, usize) { - let bytes = rustpython_host_env::ctypes::wchar_null_terminated_bytes(s); + let bytes = wchar_null_terminated_bytes(s); let ptr = bytes.as_ptr() as usize; let holder: PyObjectRef = vm.ctx.new_bytes(bytes).into(); (holder, ptr) diff --git a/crates/vm/src/stdlib/_ctypes/function.rs b/crates/vm/src/stdlib/_ctypes/function.rs index 90b41a4e66a..765ce2a2374 100644 --- a/crates/vm/src/stdlib/_ctypes/function.rs +++ b/crates/vm/src/stdlib/_ctypes/function.rs @@ -149,7 +149,7 @@ fn conv_param(value: &PyObject, vm: &VirtualMachine) -> PyResult { // 4. Python str -> wide string pointer (like PyUnicode_AsWideCharString) if let Some(s) = value.downcast_ref::() { - let wide_bytes = rustpython_host_env::ctypes::utf16z_bytes(s.as_wtf8()); + let wide_bytes: Vec = rustpython_host_env::ctypes::utf16z_bytes(s.as_wtf8()); let keep = vm.ctx.new_bytes(wide_bytes); let addr = keep.as_bytes().as_ptr() as usize; return Ok(Argument { diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index ab1be4297ec..596197c62e3 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -6182,7 +6182,10 @@ mod winconsoleio { } let name_str = nameobj.str(vm)?; - let wide = name_str.as_wtf8().to_wide_cstring(); + let wide = name_str + .as_wtf8() + .to_wide_cstring() + .map_err(|e| e.to_pyexception(vm))?; fd = host_nt::open_console_path_fd(&wide, writable) .map_err(|err| err.to_pyexception(vm))?; diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 0d54530d4b2..3d012defb2a 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -10,6 +10,7 @@ mod _winapi { builtins::PyStrRef, common::lock::PyMutex, convert::ToPyException, + exceptions::nul_char_error, function::{ArgMapping, ArgSequence, OptionalArg}, types::Constructor, windows::{WinHandle, WindowsSysResult}, @@ -92,7 +93,10 @@ mod _winapi { _template_file: PyObjectRef, // Always NULL (0) vm: &VirtualMachine, ) -> PyResult { - let file_name_wide = file_name.as_wtf8().to_wide_cstring(); + let file_name_wide = file_name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; host_winapi::create_file_w( &file_name_wide, desired_access, @@ -231,25 +235,19 @@ mod _winapi { let handle_list = get_handle_list(args.startup_info.get_attr("lpAttributeList", vm)?, vm)?; // Validate no embedded null bytes in command name and command line - // before handing the strings off; to_wide_cstring truncates at NUL. - if let Some(ref name) = args.name - && name.as_bytes().contains(&0) - { - return Err(crate::exceptions::nul_char_error(vm)); - } - if let Some(ref cmd) = args.command_line - && cmd.as_bytes().contains(&0) - { - return Err(crate::exceptions::nul_char_error(vm)); - } - - let wcstring = |s: PyStrRef| s.as_wtf8().to_wide_cstring(); - let app_name = args.name.as_ref().map(|s| wcstring(s.clone())); - let current_dir = args.current_dir.as_ref().map(|s| wcstring(s.clone())); + // before handing the strings off; to_wide_cstring rejects interior NULs. + let wcstring = |s: PyStrRef| { + s.as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm)) + }; + let app_name = args.name.map(wcstring).transpose()?; + let current_dir = args.current_dir.map(wcstring).transpose()?; let mut command_line = args .command_line - .as_ref() - .map(|s| wcstring(s.clone()).into_vec_with_nul()); + .map(|s| wcstring(s).map(widestring::WideCString::into_vec_with_nul)) + .transpose() + .map_err(|_| nul_char_error(vm))?; let procinfo = host_winapi::create_process( app_name.as_deref(), @@ -289,9 +287,12 @@ mod _winapi { } #[pyfunction] - fn NeedCurrentDirectoryForExePath(exe_name: PyStrRef) -> bool { - let exe_name = exe_name.as_wtf8().to_wide_cstring(); - host_winapi::need_current_directory_for_exe_path_w(&exe_name) + fn NeedCurrentDirectoryForExePath(exe_name: PyStrRef, vm: &VirtualMachine) -> PyResult { + exe_name + .as_wtf8() + .to_wide_cstring() + .map(|exe_name| host_winapi::need_current_directory_for_exe_path_w(&exe_name)) + .map_err(|_| nul_char_error(vm)) } #[pyfunction] @@ -404,7 +405,11 @@ mod _winapi { name: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - let name = name.flatten().map(|name| name.as_wtf8().to_wide_cstring()); + let name = name + .flatten() + .map(|name| name.as_wtf8().to_wide_cstring()) + .transpose() + .map_err(|_| nul_char_error(vm))?; host_winapi::create_job_object_w(name.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) @@ -450,8 +455,11 @@ mod _winapi { name: PyStrRef, vm: &VirtualMachine, ) -> PyResult { - let name_wide = name.as_wtf8().to_wide_cstring(); - host_winapi::open_mutex_w(desired_access, inherit_handle, &name_wide) + let name = name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + host_winapi::open_mutex_w(desired_access, inherit_handle, &name) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -494,8 +502,13 @@ mod _winapi { } // Use ToWideString which properly handles WTF-8 (including surrogates) - let locale_wide = locale.as_wtf8().to_wide_cstring(); - let src_wide = src.as_wtf8().to_wide(); + let locale_wide = locale + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + // SAFETY: Interior NULs and non-NUL capped strings are fine here because the API takes + // in a length. + let src_wide: Vec<_> = src.as_wtf8().encode_wide().collect(); if src_wide.len() > i32::MAX as usize { return Err(vm.new_overflow_error("input string is too long")); @@ -532,9 +545,13 @@ mod _winapi { /// CreateNamedPipe - Create a named pipe #[pyfunction] fn CreateNamedPipe(args: CreateNamedPipeArgs, vm: &VirtualMachine) -> PyResult { - let name_wide = args.name.as_wtf8().to_wide_cstring(); + let name = args + .name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; host_winapi::create_named_pipe_w( - &name_wide, + &name, args.open_mode, args.pipe_mode, args.max_instances, @@ -652,26 +669,33 @@ mod _winapi { /// GetShortPathName - Return the short version of the provided path. #[pyfunction] fn GetShortPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = path.as_wtf8().to_wide_cstring(); - let wide = - host_winapi::get_short_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; + let path = path + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + let wide = host_winapi::get_short_path_name_w(&path).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) } /// GetLongPathName - Return the long version of the provided path. #[pyfunction] fn GetLongPathName(path: PyStrRef, vm: &VirtualMachine) -> PyResult { - let path_wide = path.as_wtf8().to_wide_cstring(); - let wide = - host_winapi::get_long_path_name_w(&path_wide).map_err(|e| e.to_pyexception(vm))?; + let path = path + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + let wide = host_winapi::get_long_path_name_w(&path).map_err(|e| e.to_pyexception(vm))?; Ok(path_name_result_to_pystr(wide, vm)) } /// WaitNamedPipe - Wait for an instance of a named pipe to become available. #[pyfunction] fn WaitNamedPipe(name: PyStrRef, timeout: u32, vm: &VirtualMachine) -> PyResult<()> { - let name_wide = name.as_wtf8().to_wide_cstring(); - host_winapi::wait_named_pipe_w(&name_wide, timeout).map_err(|e| e.to_pyexception(vm)) + let name = name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + host_winapi::wait_named_pipe_w(&name, timeout).map_err(|e| e.to_pyexception(vm)) } /// PeekNamedPipe - Peek at data in a named pipe without removing it. @@ -724,8 +748,11 @@ mod _winapi { ) -> PyResult { let _ = security_attributes; // Ignored, always NULL - let name_wide = name.map(|n| n.as_wtf8().to_wide_cstring()); - host_winapi::create_event_w(manual_reset, initial_state, name_wide.as_deref()) + let name = name + .map(|n| n.as_wtf8().to_wide_cstring()) + .transpose() + .map_err(|_| nul_char_error(vm))?; + host_winapi::create_event_w(manual_reset, initial_state, name.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -852,8 +879,11 @@ mod _winapi { vm: &VirtualMachine, ) -> PyResult { let _ = security_attributes; - let name_wide = name.map(|n| n.as_wtf8().to_wide_cstring()); - host_winapi::create_mutex_w(initial_owner, name_wide.as_deref()) + let name = name + .map(|n| n.as_wtf8().to_wide_cstring()) + .transpose() + .map_err(|_| nul_char_error(vm))?; + host_winapi::create_mutex_w(initial_owner, name.as_deref()) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -866,8 +896,11 @@ mod _winapi { name: PyStrRef, vm: &VirtualMachine, ) -> PyResult { - let name_wide = name.as_wtf8().to_wide_cstring(); - host_winapi::open_event_w(desired_access, inherit_handle, &name_wide) + let name = name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + host_winapi::open_event_w(desired_access, inherit_handle, &name) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -965,20 +998,16 @@ mod _winapi { name: Option, vm: &VirtualMachine, ) -> PyResult { - if let Some(ref n) = name - && n.as_bytes().contains(&0) - { - return Err( - vm.new_value_error("CreateFileMapping: name must not contain null characters") - ); - } - let name_wide = name.as_ref().map(|n| n.as_wtf8().to_wide_cstring()); + let name = name + .map(|n| n.as_wtf8().to_wide_cstring()) + .transpose() + .map_err(|_| nul_char_error(vm))?; host_winapi::create_file_mapping_w( file_handle.0, protect, max_size_high, max_size_low, - name_wide.as_deref(), + name.as_deref(), ) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) @@ -992,13 +1021,11 @@ mod _winapi { name: PyStrRef, vm: &VirtualMachine, ) -> PyResult { - if name.as_bytes().contains(&0) { - return Err( - vm.new_value_error("OpenFileMapping: name must not contain null characters") - ); - } - let name_wide = name.as_wtf8().to_wide_cstring(); - host_winapi::open_file_mapping_w(desired_access, inherit_handle, &name_wide) + let name = name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + host_winapi::open_file_mapping_w(desired_access, inherit_handle, &name) .map(WinHandle) .map_err(|e| e.to_pyexception(vm)) } @@ -1044,8 +1071,14 @@ mod _winapi { _progress_routine: OptionalArg, vm: &VirtualMachine, ) -> PyResult<()> { - let src_wide = existing_file_name.as_wtf8().to_wide_cstring(); - let dst_wide = new_file_name.as_wtf8().to_wide_cstring(); + let src_wide = existing_file_name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; + let dst_wide = new_file_name + .as_wtf8() + .to_wide_cstring() + .map_err(|_| nul_char_error(vm))?; host_winapi::copy_file2(&src_wide, &dst_wide, flags).map_err(|e| e.to_pyexception(vm)) } diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs index 31a08195c58..f42af5881a0 100644 --- a/crates/vm/src/stdlib/nt.rs +++ b/crates/vm/src/stdlib/nt.rs @@ -9,9 +9,9 @@ pub(crate) mod module { Py, PyResult, TryFromObject, VirtualMachine, builtins::{PyBytes, PyDictRef, PyListRef, PyStr, PyStrRef, PyTupleRef}, convert::ToPyException, - exceptions::{self, OSErrorBuilder}, + exceptions::{self, OSErrorBuilder, ToOSErrorBuilder}, function::{ArgMapping, Either, OptionalArg}, - host_env::{crt_fd, windows::ToWideString}, + host_env::crt_fd, ospath::{OsPath, OsPathOrFd}, stdlib::os::{_os, DirFd, SupportFunc, TargetIsDirectory}, }; @@ -72,8 +72,8 @@ pub(crate) mod module { vm: &VirtualMachine, ) -> PyResult<()> { let [] = dir_fd.0; - let _ = path.to_wide_cstring(vm)?; - host_nt::remove(path.as_ref()).map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) + let wide = path.to_wide_cstring(vm)?; + host_nt::remove(&wide).map_err(|err| OSErrorBuilder::with_filename(&err, path, vm)) } #[pyfunction] @@ -93,7 +93,6 @@ pub(crate) mod module { #[pyfunction] pub(super) fn symlink(args: SymlinkArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { - use crate::exceptions::ToOSErrorBuilder; let src = args.src.to_wide_cstring(vm)?; let dst = args.dst.to_wide_cstring(vm)?; if let Err(err) = host_nt::symlink( @@ -164,7 +163,8 @@ pub(crate) mod module { } fn win32_lchmod(path: &OsPath, mode: u32, vm: &VirtualMachine) -> PyResult<()> { - host_nt::win32_lchmod(path.path.as_os_str(), mode, S_IWRITE) + let wide = path.to_wide_cstring(vm)?; + host_nt::win32_lchmod(&wide, mode, S_IWRITE) .map_err(|err| OSErrorBuilder::with_filename(&err, path.clone(), vm)) } @@ -212,7 +212,8 @@ pub(crate) mod module { /// Uses FindFirstFileW to get the name as stored on the filesystem. #[pyfunction] fn _findfirstfile(path: OsPath, vm: &VirtualMachine) -> PyResult { - let filename = host_nt::find_first_file_name(path.as_ref()) + let wide = path.to_wide_cstring(vm)?; + let filename = host_nt::find_first_file_name(&wide) .map_err(|err| OSErrorBuilder::with_filename(&err, path.clone(), vm))?; let filename_str = filename .to_str() @@ -274,7 +275,7 @@ pub(crate) mod module { } /// _testFileTypeByName - test file type by path name - fn _test_file_type_by_name(path: &std::path::Path, tested_type: u32) -> bool { + fn _test_file_type_by_name(path: &widestring::WideCStr, tested_type: u32) -> bool { let tested_type = match tested_type { PY_IFREG => host_nt::TestType::RegularFile, PY_IFDIR => host_nt::TestType::Directory, @@ -287,11 +288,6 @@ pub(crate) mod module { host_nt::test_file_type_by_name(path, tested_type) } - /// _testFileExistsByName - test if path exists - fn _test_file_exists_by_name(path: &std::path::Path, follow_links: bool) -> bool { - host_nt::test_file_exists_by_name(path, follow_links) - } - /// _testFileType wrapper - handles both fd and path fn _test_file_type(path_or_fd: &OsPathOrFd<'_>, tested_type: u32) -> bool { match path_or_fd { @@ -303,7 +299,8 @@ pub(crate) mod module { false } } - OsPathOrFd::Path(path) => _test_file_type_by_name(path.as_ref(), tested_type), + OsPathOrFd::Path(path) => widestring::WideCString::from_os_str(&path.path) + .is_ok_and(|path| _test_file_type_by_name(&path, tested_type)), } } @@ -311,7 +308,8 @@ pub(crate) mod module { fn _test_file_exists(path_or_fd: &OsPathOrFd<'_>, follow_links: bool) -> bool { match path_or_fd { OsPathOrFd::Fd(fd) => host_nt::fd_exists(*fd), - OsPathOrFd::Path(path) => _test_file_exists_by_name(path.as_ref(), follow_links), + OsPathOrFd::Path(path) => widestring::WideCString::from_os_str(&path.path) + .is_ok_and(|path| host_nt::test_file_exists_by_name(&path, follow_links)), } } @@ -366,8 +364,8 @@ pub(crate) mod module { /// Check if a path is on a Windows Dev Drive. #[pyfunction] fn _path_isdevdrive(path: OsPath, vm: &VirtualMachine) -> PyResult { - let _ = path.to_wide_cstring(vm)?; - host_nt::path_isdevdrive(path.as_ref()).map_err(|err| err.to_pyexception(vm)) + let path = path.to_wide_cstring(vm)?; + host_nt::path_isdevdrive(&path).map_err(|err| err.to_pyexception(vm)) } #[cfg(target_env = "msvc")] @@ -575,16 +573,16 @@ pub(crate) mod module { #[pyfunction] fn _getfinalpathname(path: OsPath, vm: &VirtualMachine) -> PyResult { - let _ = path.to_wide_cstring(vm)?; - let final_path = host_nt::getfinalpathname(path.as_ref()) + let wide = path.to_wide_cstring(vm)?; + let final_path = host_nt::getfinalpathname(&wide) .map_err(|err| OSErrorBuilder::with_filename(&err, path.clone(), vm))?; Ok(path.mode().process_path(final_path, vm)) } #[pyfunction] fn _getfullpathname(path: OsPath, vm: &VirtualMachine) -> PyResult { - let _ = path.to_wide_cstring(vm)?; - let buffer = host_nt::getfullpathname(path.as_ref()) + let wide = path.to_wide_cstring(vm)?; + let buffer = host_nt::getfullpathname(&wide) .map_err(|err| OSErrorBuilder::with_filename(&err, path.clone(), vm))?; Ok(path.mode().process_path(buffer, vm)) } @@ -596,7 +594,7 @@ pub(crate) mod module { if buflen > u32::MAX as usize { return Err(vm.new_overflow_error("path too long")); } - let buffer = host_nt::getvolumepathname(path.as_ref()) + let buffer = host_nt::getvolumepathname(&wide) .map_err(|err| OSErrorBuilder::with_filename(&err, path.clone(), vm))?; Ok(path.mode().process_path(buffer, vm)) } @@ -745,10 +743,12 @@ pub(crate) mod module { } #[pyfunction] - fn _path_splitroot(path: OsPath, _vm: &VirtualMachine) -> (Wtf8Buf, Wtf8Buf) { - let orig: Vec<_> = path.path.to_wide(); + fn _path_splitroot(path: OsPath, vm: &VirtualMachine) -> PyResult<(Wtf8Buf, Wtf8Buf)> { + let orig: Vec<_> = widestring::WideCString::from_os_str(path.path) + .map_err(|e| e.to_pyexception(vm))? + .into_vec(); if orig.is_empty() { - return (Wtf8Buf::new(), Wtf8Buf::new()); + return Ok((Wtf8Buf::new(), Wtf8Buf::new())); } let backslashed: Vec<_> = orig .iter() @@ -757,8 +757,8 @@ pub(crate) mod module { .chain(core::iter::once(0)) // null-terminated .collect(); - let backslashed_wide = widestring::WideCStr::from_slice_truncate(&backslashed) - .expect("backslashed is null-terminated"); + let backslashed_wide = widestring::WideCStr::from_slice(&backslashed) + .expect("backslashed is null-terminated and does not contain interior nulls"); if let Some(len) = host_nt::path_skip_root(backslashed_wide) { assert!( len < backslashed.len(), // backslashed is null-terminated @@ -768,15 +768,15 @@ pub(crate) mod module { backslashed.len() ); if len != 0 { - ( + Ok(( Wtf8Buf::from_wide(&orig[..len]), Wtf8Buf::from_wide(&orig[len..]), - ) + )) } else { - (Wtf8Buf::from_wide(&orig), Wtf8Buf::new()) + Ok((Wtf8Buf::from_wide(&orig), Wtf8Buf::new())) } } else { - (Wtf8Buf::new(), Wtf8Buf::from_wide(&orig)) + Ok((Wtf8Buf::new(), Wtf8Buf::from_wide(&orig))) } } @@ -947,8 +947,8 @@ pub(crate) mod module { #[pyfunction] fn _getdiskusage(path: OsPath, vm: &VirtualMachine) -> PyResult<(u64, u64)> { - let _ = path.to_wide_cstring(vm)?; - host_nt::getdiskusage(path.as_ref()).map_err(|err| err.to_pyexception(vm)) + let path = path.to_wide_cstring(vm)?; + host_nt::getdiskusage(&path).map_err(|err| err.to_pyexception(vm)) } #[pyfunction] @@ -994,8 +994,8 @@ pub(crate) mod module { #[pyfunction] fn listmounts(volume: OsPath, vm: &VirtualMachine) -> PyResult { - let _ = volume.to_wide_cstring(vm)?; - let result = host_nt::listmounts(volume.as_ref()) + let volume = volume.to_wide_cstring(vm)?; + let result = host_nt::listmounts(&volume) .map_err(|err| err.to_pyexception(vm))? .into_iter() .map(|mount| vm.new_pyobj(mount.to_string_lossy().into_owned())) @@ -1069,10 +1069,11 @@ pub(crate) mod module { #[pyfunction] fn readlink(path: OsPath, vm: &VirtualMachine) -> PyResult { let mode = path.mode(); - match host_nt::readlink(path.as_ref()) { + let wide = path.to_wide_cstring(vm)?; + match host_nt::readlink(&wide) { Ok(result_path) => Ok(mode.process_path(std::path::PathBuf::from(result_path), vm)), Err(host_nt::ReadlinkError::Io(err)) => { - Err(OSErrorBuilder::with_filename(&err, path.clone(), vm)) + Err(OSErrorBuilder::with_filename(&err, path, vm)) } Err(err) => Err(err.to_pyexception(vm)), } diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 9156c9fc0bf..dee7b066409 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -218,7 +218,8 @@ pub(super) mod _os { use crossbeam_utils::atomic::AtomicCell; use rustpython_common::wtf8::Wtf8Buf; #[cfg(windows)] - use rustpython_host_env::nt as host_nt; + use rustpython_host_env::{nt as host_nt, windows::ToWideString}; + #[cfg(all(any(unix, target_os = "wasi"), not(target_os = "redox")))] use rustpython_host_env::posix as host_posix; use std::{fs, io, path::PathBuf, time::SystemTime}; @@ -873,7 +874,9 @@ pub(super) mod _os { #[cfg(windows)] #[pymethod] fn is_junction(&self, _vm: &VirtualMachine) -> bool { - host_nt::test_file_type_by_name(&self.pathval, host_nt::TestType::Junction) + self.pathval.to_wide_cstring().is_ok_and(|path| { + host_nt::test_file_type_by_name(&path, host_nt::TestType::Junction) + }) } #[pymethod] @@ -1007,8 +1010,8 @@ pub(super) mod _os { #[cfg(windows)] let lstat = { let cell = OnceCell::new(); - if let Ok(stat_struct) = - host_nt::win32_xstat(pathval.as_os_str(), false) + if let Ok(wide) = pathval.as_os_str().to_wide_cstring() + && let Ok(stat_struct) = host_nt::win32_xstat(&wide, false) { let stat_obj = StatResultData::from_stat(&stat_struct, vm).to_pyobject(vm); @@ -1350,7 +1353,10 @@ pub(super) mod _os { ) -> io::Result> { let [] = dir_fd.0; match file { - OsPathOrFd::Path(path) => host_nt::win32_xstat(&path.path, follow_symlinks.0), + OsPathOrFd::Path(path) => { + let path = path.path.to_wide_cstring()?; + host_nt::win32_xstat(&path, follow_symlinks.0) + } OsPathOrFd::Fd(fd) => crate::host_env::fileutils::fstat(fd), } .map(Some) diff --git a/crates/vm/src/stdlib/winreg.rs b/crates/vm/src/stdlib/winreg.rs index 468767e9d38..df3959acdab 100644 --- a/crates/vm/src/stdlib/winreg.rs +++ b/crates/vm/src/stdlib/winreg.rs @@ -8,8 +8,8 @@ mod winreg { use crate::builtins::{PyInt, PyStr, PyTuple, PyTypeRef}; use crate::common::hash::PyHash; use crate::convert::{ToPyException, TryFromObject}; + use crate::exceptions::nul_char_error; use crate::function::FuncArgs; - use crate::host_env::windows::ToWideString; use crate::object::AsObject; use crate::protocol::PyNumberMethods; use crate::types::{AsNumber, Hashable}; @@ -18,7 +18,9 @@ mod winreg { use crossbeam_utils::atomic::AtomicCell; use malachite_bigint::Sign; use num_traits::ToPrimitive; + use rustpython_host_env::windows::ToWideString; use rustpython_host_env::winreg as host_winreg; + use widestring::{WideCString, WideString}; /// Atomic HKEY handle type for lock-free thread-safe access type AtomicHKEY = AtomicCell; @@ -255,14 +257,13 @@ mod winreg { key: PyRef, vm: &VirtualMachine, ) -> PyResult { - let wide_computer_name = computer_name.map(|n| n.to_wide_cstring()); + let computer_name = computer_name + .map(WideCString::from_str) + .transpose() + .map_err(|e| e.to_pyexception(vm))?; let mut ret_key = core::ptr::null_mut(); let res = unsafe { - host_winreg::connect_registry( - wide_computer_name.as_deref(), - key.hkey.load(), - &mut ret_key, - ) + host_winreg::connect_registry(computer_name.as_deref(), key.hkey.load(), &mut ret_key) }; if res == 0 { Ok(PyHkey::new(ret_key)) @@ -273,9 +274,9 @@ mod winreg { #[pyfunction] fn CreateKey(key: PyRef, sub_key: String, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = sub_key.to_wide_cstring(); + let sub_key = WideCString::from_str(sub_key).map_err(|e| e.to_pyexception(vm))?; let mut out_key = core::ptr::null_mut(); - let res = unsafe { host_winreg::create_key(key.hkey.load(), &wide_sub_key, &mut out_key) }; + let res = unsafe { host_winreg::create_key(key.hkey.load(), &sub_key, &mut out_key) }; if res == 0 { Ok(PyHkey::new(out_key)) } else { @@ -297,7 +298,7 @@ mod winreg { #[pyfunction] fn CreateKeyEx(args: CreateKeyExArgs, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = args.sub_key.to_wide_cstring(); + let wide_sub_key = WideCString::from_str(args.sub_key).map_err(|e| e.to_pyexception(vm))?; let mut res: host_winreg::HKEY = core::ptr::null_mut(); let err = unsafe { let key = args.key.hkey.load(); @@ -330,8 +331,8 @@ mod winreg { #[pyfunction] fn DeleteKey(key: PyRef, sub_key: String, vm: &VirtualMachine) -> PyResult<()> { - let wide_sub_key = sub_key.to_wide_cstring(); - let res = unsafe { host_winreg::delete_key(key.hkey.load(), &wide_sub_key) }; + let sub_key = WideCString::from_str(sub_key).map_err(|e| e.to_pyexception(vm))?; + let res = unsafe { host_winreg::delete_key(key.hkey.load(), &sub_key) }; if res == 0 { Ok(()) } else { @@ -341,7 +342,10 @@ mod winreg { #[pyfunction] fn DeleteValue(key: PyRef, value: Option, vm: &VirtualMachine) -> PyResult<()> { - let wide_value = value.map(|v| v.to_wide_cstring()); + let wide_value = value + .map(WideCString::from_str) + .transpose() + .map_err(|e| e.to_pyexception(vm))?; let res = unsafe { host_winreg::delete_value(key.hkey.load(), wide_value.as_deref()) }; if res == 0 { Ok(()) @@ -364,7 +368,7 @@ mod winreg { #[pyfunction] fn DeleteKeyEx(args: DeleteKeyExArgs, vm: &VirtualMachine) -> PyResult<()> { - let wide_sub_key = args.sub_key.to_wide_cstring(); + let wide_sub_key = WideCString::from_str(args.sub_key).map_err(|e| e.to_pyexception(vm))?; let res = unsafe { host_winreg::delete_key_ex( args.key.hkey.load(), @@ -501,8 +505,12 @@ mod winreg { file_name: String, vm: &VirtualMachine, ) -> PyResult<()> { - let sub_key = sub_key.to_wide_cstring(); - let file_name = file_name.to_wide_cstring(); + let (Ok(sub_key), Ok(file_name)) = ( + WideCString::from_str(sub_key), + WideCString::from_str(file_name), + ) else { + return Err(nul_char_error(vm)); + }; let res = unsafe { host_winreg::load_key(key.hkey.load(), &sub_key, &file_name) }; if res == 0 { Ok(()) @@ -526,11 +534,11 @@ mod winreg { #[pyfunction] #[pyfunction(name = "OpenKeyEx")] fn OpenKey(args: OpenKeyArgs, vm: &VirtualMachine) -> PyResult { - let wide_sub_key = args.sub_key.to_wide_cstring(); + let sub_key = WideCString::from_str(args.sub_key).map_err(|e| e.to_pyexception(vm))?; let mut res: host_winreg::HKEY = core::ptr::null_mut(); let err = unsafe { let key = args.key.hkey.load(); - host_winreg::open_key_ex(key, &wide_sub_key, args.reserved, args.access, &mut res) + host_winreg::open_key_ex(key, &sub_key, args.reserved, args.access, &mut res) }; if err == 0 { Ok(PyHkey { @@ -566,14 +574,19 @@ mod winreg { )); } - host_winreg::query_default_value(hkey, sub_key.as_deref().map(std::ffi::OsStr::new)) + let sub_key = sub_key + .map(WideCString::from_str) + .transpose() + .map_err(|e| e.to_pyexception(vm))?; + host_winreg::query_default_value(hkey, sub_key.as_deref()) .map_err(|err| err.to_pyexception(vm)) } #[pyfunction] fn QueryValueEx(key: HKEYArg, name: String, vm: &VirtualMachine) -> PyResult> { let hkey = key.0; - let (ret_buf, typ) = host_winreg::query_value_bytes(hkey, std::ffi::OsStr::new(&name)) + let wide_name = WideCString::from_str(name).map_err(|e| e.to_pyexception(vm))?; + let (ret_buf, typ) = host_winreg::query_value_bytes(hkey, &wide_name) .map_err(|err| os_error_from_windows_code(vm, err as i32))?; let obj = reg_to_py(vm, &ret_buf, typ)?; // Return tuple (value, type) @@ -582,7 +595,7 @@ mod winreg { #[pyfunction] fn SaveKey(key: PyRef, file_name: String, vm: &VirtualMachine) -> PyResult<()> { - let file_name = file_name.to_wide_cstring(); + let file_name = WideCString::from_str(file_name).map_err(|e| e.to_pyexception(vm))?; let res = unsafe { host_winreg::save_key(key.hkey.load(), &file_name) }; if res == 0 { Ok(()) @@ -611,12 +624,12 @@ mod winreg { )); } - let res = host_winreg::set_default_value( - hkey, - std::ffi::OsStr::new(&sub_key), - typ, - std::ffi::OsStr::new(&value), - ); + let sub_key = WideCString::from_str(sub_key).map_err(|e| e.to_pyexception(vm))?; + // Value can contain interior NULs. + let mut wide_value = WideString::with_capacity(value.len() + 1); + wide_value.push_str(&value); + wide_value.push_str("\0"); + let res = host_winreg::set_default_value(hkey, &sub_key, typ, &wide_value); if res == 0 { Ok(()) @@ -738,12 +751,15 @@ mod winreg { // Return empty string as UTF-16 null terminator return Ok(Some(vec![0u8, 0u8])); } - let s = value + // Registry values are allowed to contain interior NULs. + let bytes: Vec = value .downcast::() - .map_err(|_| vm.new_type_error("value must be a string"))?; - let wide = s.as_wtf8().to_wide_with_nul(); - // Convert Vec to Vec - let bytes: Vec = wide.iter().flat_map(|&c| c.to_le_bytes()).collect(); + .map_err(|_| vm.new_type_error("value must be a string"))? + .as_wtf8() + .encode_wide() + .chain([0u16]) + .flat_map(u16::to_le_bytes) + .collect(); Ok(Some(bytes)) } REG_MULTI_SZ => { @@ -755,16 +771,26 @@ mod winreg { .downcast::() .map_err(|_| vm.new_type_error("value must be a list of strings"))?; - let mut bytes: Vec = Vec::new(); + let mut encoded = Vec::new(); for item in list.borrow_vec().iter() { + // The final vector is a list of NUL terminated strings. The list itself is + // NUL terminated as well. Unlike REG_SZ, interior NULs are forbidden because + // it would truncate the list. + // https://learn.microsoft.com/en-us/windows/win32/sysinfo/registry-value-types let s = item .downcast_ref::() - .ok_or_else(|| vm.new_type_error("list items must be strings"))?; - let wide = s.as_wtf8().to_wide_with_nul(); - bytes.extend(wide.iter().flat_map(|&c| c.to_le_bytes())); + .ok_or_else(|| vm.new_type_error("list items must be strings"))? + .as_wtf8() + .to_wide_cstring() + .map(WideCString::into_vec_with_nul) + .map_err(|e| e.to_pyexception(vm))?; + encoded.extend(s); } - // Add final null terminator (double null at end) - bytes.extend([0u8, 0u8]); + let bytes = encoded + .into_iter() + .flat_map(u16::to_le_bytes) + .chain(0u16.to_le_bytes()) + .collect(); Ok(Some(bytes)) } // REG_BINARY and other types @@ -793,14 +819,17 @@ mod winreg { value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - let wide_value_name = value_name.as_deref().map(|s| s.to_wide_cstring()); + let value_name = value_name + .map(WideCString::from_str) + .transpose() + .map_err(|e| e.to_pyexception(vm))?; let reg_value = py2reg(value, typ, vm)?; let (ptr, len) = match ®_value { Some(v) => (v.as_ptr(), v.len() as u32), None => (core::ptr::null(), 0), }; let res = unsafe { - host_winreg::set_value_ex(key.hkey.load(), wide_value_name.as_deref(), typ, ptr, len) + host_winreg::set_value_ex(key.hkey.load(), value_name.as_deref(), typ, ptr, len) }; if res != 0 { return Err(os_error_from_windows_code(vm, res as i32)); @@ -841,7 +870,7 @@ mod winreg { #[pyfunction] fn ExpandEnvironmentStrings(i: String, vm: &VirtualMachine) -> PyResult { - host_winreg::expand_environment_strings(std::ffi::OsStr::new(&i)) - .map_err(|err| err.to_pyexception(vm)) + let i = WideCString::from_str(i).map_err(|err| err.to_pyexception(vm))?; + host_winreg::expand_environment_strings(&i).map_err(|err| err.to_pyexception(vm)) } } diff --git a/crates/vm/src/stdlib/winsound.rs b/crates/vm/src/stdlib/winsound.rs index 95032ad8970..61bf24a998a 100644 --- a/crates/vm/src/stdlib/winsound.rs +++ b/crates/vm/src/stdlib/winsound.rs @@ -7,8 +7,6 @@ pub(crate) use winsound::module_def; mod winsound { use crate::builtins::{PyBaseExceptionRef, PyBytes, PyStr}; use crate::convert::{IntoPyException, ToPyException, TryFromBorrowedObject}; - use crate::exceptions; - use crate::host_env::windows::ToWideString; use crate::protocol::PyBuffer; use crate::{AsObject, PyObjectRef, PyResult, VirtualMachine}; use rustpython_host_env::winsound::{PlaySoundError, PlaySoundSource, play_sound}; @@ -106,7 +104,12 @@ mod winsound { // os.fspath(sound) let path = match sound.downcast_ref::() { - Some(s) => s.as_wtf8().to_owned(), + Some(s) => { + let s = s.as_wtf8(); + let mut buf = Vec::with_capacity(s.len() + 1); + buf.extend(s.encode_wide()); + buf + } None => { let fspath = vm.get_method_or_type_error( sound.clone(), @@ -129,27 +132,27 @@ mod winsound { return Err(vm.new_type_error("'sound' must resolve to str, not bytes")); } - let s: &PyStr = result.downcast_ref().ok_or_else(|| { - vm.new_type_error(format!( - "expected {}.__fspath__() to return str or bytes, not {}", - sound.class().name(), - result.class().name() - )) - })?; - - s.as_wtf8().to_owned() + let s = result + .downcast_ref::() + .ok_or_else(|| { + vm.new_type_error(format!( + "expected {}.__fspath__() to return str or bytes, not {}", + sound.class().name(), + result.class().name() + )) + })? + .as_wtf8(); + + let mut buf = Vec::with_capacity(s.len() + 1); + buf.extend(s.encode_wide()); + buf } }; // Check for embedded null characters - if path.as_bytes().contains(&0) { - return Err(exceptions::nul_char_error(vm)); - } - - let wide = path.to_wide_with_nul(); let wide_cstr = - widestring::WideCStr::from_slice_truncate(&wide).map_err(|e| e.to_pyexception(vm))?; - play_sound(PlaySoundSource::Name(wide_cstr), flags).map_err(map_play_err(vm)) + widestring::WideCString::from_vec(path).map_err(|e| e.to_pyexception(vm))?; + play_sound(PlaySoundSource::Name(&wide_cstr), flags).map_err(map_play_err(vm)) } #[derive(FromArgs)]