diff --git a/library/std/src/sys/fs/windows/remove_dir_all.rs b/library/std/src/sys/fs/windows/remove_dir_all.rs index 06734f9e3097b..c8b1a07676855 100644 --- a/library/std/src/sys/fs/windows/remove_dir_all.rs +++ b/library/std/src/sys/fs/windows/remove_dir_all.rs @@ -33,7 +33,7 @@ use core::sync::atomic::{Atomic, AtomicU32, Ordering}; use super::{AsRawHandle, DirBuff, File, FromRawHandle}; use crate::sys::c; -use crate::sys::pal::api::WinError; +use crate::sys::pal::api::{UnicodeStrRef, WinError, unicode_str}; use crate::thread; // The maximum number of times to spin when waiting for deletes to complete. @@ -74,7 +74,7 @@ unsafe fn nt_open_file( /// `options` will be OR'd with `FILE_OPEN_REPARSE_POINT`. fn open_link_no_reparse( parent: &File, - path: &[u16], + path: UnicodeStrRef<'_>, access: u32, options: u32, ) -> Result, WinError> { @@ -90,9 +90,8 @@ fn open_link_no_reparse( static ATTRIBUTES: Atomic = AtomicU32::new(c::OBJ_DONT_REPARSE); let result = unsafe { - let mut path_str = c::UNICODE_STRING::from_ref(path); let mut object = c::OBJECT_ATTRIBUTES { - ObjectName: &mut path_str, + ObjectName: path.as_ptr(), RootDirectory: parent.as_raw_handle(), Attributes: ATTRIBUTES.load(Ordering::Relaxed), ..c::OBJECT_ATTRIBUTES::with_length() @@ -129,7 +128,7 @@ fn open_link_no_reparse( } } -fn open_dir(parent: &File, name: &[u16]) -> Result, WinError> { +fn open_dir(parent: &File, name: UnicodeStrRef<'_>) -> Result, WinError> { // Open the directory for synchronous directory listing. open_link_no_reparse( parent, @@ -140,7 +139,7 @@ fn open_dir(parent: &File, name: &[u16]) -> Result, WinError> { ) } -fn delete(parent: &File, name: &[u16]) -> Result<(), WinError> { +fn delete(parent: &File, name: UnicodeStrRef<'_>) -> Result<(), WinError> { // Note that the `delete` function consumes the opened file to ensure it's // dropped immediately. See module comments for why this is important. match open_link_no_reparse(parent, name, c::DELETE, 0) { @@ -179,8 +178,9 @@ pub fn remove_dir_all_iterative(dir: File) -> Result<(), WinError> { 'outer: while let Some(dir) = dirlist.pop() { let more_data = dir.fill_dir_buff(&mut buffer, restart)?; for (name, is_directory) in buffer.iter() { + let name = unicode_str!(&name); if is_directory { - let Some(subdir) = open_dir(&dir, &name)? else { continue }; + let Some(subdir) = open_dir(&dir, name)? else { continue }; dirlist.push(dir); dirlist.push(subdir); continue 'outer; @@ -188,7 +188,7 @@ pub fn remove_dir_all_iterative(dir: File) -> Result<(), WinError> { // Attempt to delete, retrying on sharing violation errors as these // can often be very temporary. E.g. if something takes just a // bit longer than expected to release a file handle. - retry(|| delete(&dir, &name), WinError::SHARING_VIOLATION)?; + retry(|| delete(&dir, name), WinError::SHARING_VIOLATION)?; } } if more_data { @@ -197,7 +197,8 @@ pub fn remove_dir_all_iterative(dir: File) -> Result<(), WinError> { } else { // Attempt to delete, retrying on not empty errors because we may // need to wait some time for files to be removed from the filesystem. - retry(|| delete(&dir, &[]), WinError::DIR_NOT_EMPTY)?; + let name = unicode_str!(""); + retry(|| delete(&dir, name), WinError::DIR_NOT_EMPTY)?; restart = true; } } diff --git a/library/std/src/sys/pal/windows/api.rs b/library/std/src/sys/pal/windows/api.rs index 773455c572f70..25a6c2d7d8eda 100644 --- a/library/std/src/sys/pal/windows/api.rs +++ b/library/std/src/sys/pal/windows/api.rs @@ -30,6 +30,7 @@ //! should go in sys/pal/windows/mod.rs rather than here. See `IoResult` as an example. use core::ffi::c_void; +use core::marker::PhantomData; use super::c; @@ -291,3 +292,75 @@ impl WinError { pub const TIMEOUT: Self = Self::new(c::ERROR_TIMEOUT); // tidy-alphabetical-end } + +/// A wrapper around a UNICODE_STRING that is equivalent to `&[u16]`. +/// +/// It is preferable to use the `unicode_str!` macro as that contains mitigations for #143078. +/// +/// If the MaximumLength field of the underlying UNICODE_STRING is greater than +/// the Length field then you can test if the string is null terminated by inspecting +/// the u16 directly after the string. You cannot otherwise depend on nul termination. +#[derive(Copy, Clone)] +pub struct UnicodeStrRef<'a> { + s: c::UNICODE_STRING, + lifetime: PhantomData<&'a [u16]>, +} + +static EMPTY_STRING_NULL_TERMINATED: &[u16] = &[0]; + +impl UnicodeStrRef<'_> { + const fn new(slice: &[u16], is_null_terminated: bool) -> Self { + let (len, max_len, ptr) = if slice.is_empty() { + (0, 2, EMPTY_STRING_NULL_TERMINATED.as_ptr().cast_mut()) + } else { + let len = slice.len() - (is_null_terminated as usize); + (len * 2, size_of_val(slice), slice.as_ptr().cast_mut()) + }; + Self { + s: c::UNICODE_STRING { Length: len as _, MaximumLength: max_len as _, Buffer: ptr }, + lifetime: PhantomData, + } + } + + pub const fn from_slice_with_nul(slice: &[u16]) -> Self { + if !slice.is_empty() { + debug_assert!(slice[slice.len() - 1] == 0); + } + Self::new(slice, true) + } + + pub const fn from_slice(slice: &[u16]) -> Self { + Self::new(slice, false) + } + + /// Returns a pointer to the underlying UNICODE_STRING + pub const fn as_ptr(&self) -> *const c::UNICODE_STRING { + &self.s + } +} + +/// Create a UnicodeStringRef from a literal str or a u16 array. +/// +/// To mitigate #143078, when using a literal str the created UNICODE_STRING +/// will be nul terminated. The MaximumLength field of the UNICODE_STRING will +/// be set greater than the Length field to indicate that a nul may be present. +/// +/// If using a u16 array, the array is used exactly as provided and you cannot +/// count on the string being nul terminated. +/// This should generally be used for strings that come from the OS. +/// +/// **NOTE:** we lack a UNICODE_STRING builder type as we don't currently have +/// a use for it. If needing to dynamically build a UNICODE_STRING, the builder +/// should try to ensure there's a nul one past the end of the string. +pub macro unicode_str { + ($str:literal) => {const { + crate::sys::pal::windows::api::UnicodeStrRef::from_slice_with_nul( + crate::sys::pal::windows::api::wide_str!($str), + ) + }}, + ($array:expr) => { + crate::sys::pal::windows::api::UnicodeStrRef::from_slice( + $array, + ) + } +} diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/windows/c.rs index 53dec105d0c85..eee169d410a9c 100644 --- a/library/std/src/sys/pal/windows/c.rs +++ b/library/std/src/sys/pal/windows/c.rs @@ -37,13 +37,6 @@ pub fn nt_success(status: NTSTATUS) -> bool { status >= 0 } -impl UNICODE_STRING { - pub fn from_ref(slice: &[u16]) -> Self { - let len = size_of_val(slice); - Self { Length: len as _, MaximumLength: len as _, Buffer: slice.as_ptr() as _ } - } -} - impl OBJECT_ATTRIBUTES { pub fn with_length() -> Self { Self { diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index bc5d05c45052a..b5ccf037a4f22 100644 --- a/library/std/src/sys/pal/windows/pipe.rs +++ b/library/std/src/sys/pal/windows/pipe.rs @@ -1,9 +1,8 @@ use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; use crate::ops::Neg; use crate::os::windows::prelude::*; -use crate::sys::api::utf16; -use crate::sys::c; use crate::sys::handle::Handle; +use crate::sys::{api, c}; use crate::sys_common::{FromInner, IntoInner}; use crate::{mem, ptr}; @@ -73,8 +72,8 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res // Open a handle to the pipe filesystem (`\??\PIPE\`). // This will be used when creating a new annon pipe. let pipe_fs = { - let path = c::UNICODE_STRING::from_ref(utf16!(r"\??\PIPE\")); - object_attributes.ObjectName = &path; + let path = api::unicode_str!(r"\??\PIPE\"); + object_attributes.ObjectName = path.as_ptr(); let mut pipe_fs = ptr::null_mut(); let status = c::NtOpenFile( &mut pipe_fs, @@ -93,8 +92,12 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res // From now on we're using handles instead of paths to create and open pipes. // So set the `ObjectName` to a zero length string. + // As a (perhaps overzealous) mitigation for #143078, we use the null pointer + // for empty.Buffer instead of unicode_str!(""). + // There's no difference to the OS itself but it's possible that third party + // DLLs which hook in to processes could be relying on the exact form of this string. let empty = c::UNICODE_STRING::default(); - object_attributes.ObjectName = ∅ + object_attributes.ObjectName = &raw const empty; // Create our side of the pipe for async access. let ours = {