Skip to content

Commit 19f54cb

Browse files
committed
Workaround for mem safety in third party dlls
1 parent df32e15 commit 19f54cb

File tree

1 file changed

+41
-5
lines changed

1 file changed

+41
-5
lines changed

library/std/src/sys/fs/windows/remove_dir_all.rs

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ unsafe fn nt_open_file(
7474
/// `options` will be OR'd with `FILE_OPEN_REPARSE_POINT`.
7575
fn open_link_no_reparse(
7676
parent: &File,
77-
path: &[u16],
77+
path: &UnicodeStringNZ,
7878
access: u32,
7979
options: u32,
8080
) -> Result<Option<File>, WinError> {
@@ -90,7 +90,7 @@ fn open_link_no_reparse(
9090
static ATTRIBUTES: Atomic<u32> = AtomicU32::new(c::OBJ_DONT_REPARSE);
9191

9292
let result = unsafe {
93-
let mut path_str = c::UNICODE_STRING::from_ref(path);
93+
let mut path_str = path.as_unicode_string();
9494
let mut object = c::OBJECT_ATTRIBUTES {
9595
ObjectName: &mut path_str,
9696
RootDirectory: parent.as_raw_handle(),
@@ -129,7 +129,7 @@ fn open_link_no_reparse(
129129
}
130130
}
131131

132-
fn open_dir(parent: &File, name: &[u16]) -> Result<Option<File>, WinError> {
132+
fn open_dir(parent: &File, name: &UnicodeStringNZ) -> Result<Option<File>, WinError> {
133133
// Open the directory for synchronous directory listing.
134134
open_link_no_reparse(
135135
parent,
@@ -140,7 +140,7 @@ fn open_dir(parent: &File, name: &[u16]) -> Result<Option<File>, WinError> {
140140
)
141141
}
142142

143-
fn delete(parent: &File, name: &[u16]) -> Result<(), WinError> {
143+
fn delete(parent: &File, name: &UnicodeStringNZ) -> Result<(), WinError> {
144144
// Note that the `delete` function consumes the opened file to ensure it's
145145
// dropped immediately. See module comments for why this is important.
146146
match open_link_no_reparse(parent, name, c::DELETE, 0) {
@@ -171,14 +171,49 @@ fn retry<T: PartialEq>(
171171
}
172172
}
173173

174+
/// A UNICODE_STRING that's guarenteed to have null after the end of the string.
175+
///
176+
/// Mitigation for #143078.
177+
/// It does not prevent interior nulls but ensures any software intercepting Windows API calls
178+
/// and assuming null-termination will at least not have a memory safety issue.
179+
struct UnicodeStringNZ {
180+
s: Vec<u16>,
181+
}
182+
impl UnicodeStringNZ {
183+
pub fn new() -> Self {
184+
// This should be large enough to prevent reallocation because filenames
185+
// are by convention limited to 255 u16s.
186+
let mut s = Vec::with_capacity(256);
187+
s.push(0);
188+
Self { s }
189+
}
190+
191+
pub fn as_unicode_string(&self) -> c::UNICODE_STRING {
192+
let mut u = c::UNICODE_STRING::from_ref(&self.s);
193+
// truncate the length to before the null
194+
// note: the length is in bytes and is not a count of u16s.
195+
u.Length -= 2;
196+
u
197+
}
198+
199+
pub fn set(&mut self, s: &[u16]) -> &Self {
200+
self.s.truncate(0);
201+
self.s.extend_from_slice(s);
202+
self.s.push(0);
203+
self
204+
}
205+
}
206+
174207
pub fn remove_dir_all_iterative(dir: File) -> Result<(), WinError> {
175208
let mut buffer = DirBuff::new();
176209
let mut dirlist = vec![dir];
210+
let mut string_buffer = UnicodeStringNZ::new();
177211

178212
let mut restart = true;
179213
'outer: while let Some(dir) = dirlist.pop() {
180214
let more_data = dir.fill_dir_buff(&mut buffer, restart)?;
181215
for (name, is_directory) in buffer.iter() {
216+
let name = string_buffer.set(&name);
182217
if is_directory {
183218
let Some(subdir) = open_dir(&dir, &name)? else { continue };
184219
dirlist.push(dir);
@@ -197,7 +232,8 @@ pub fn remove_dir_all_iterative(dir: File) -> Result<(), WinError> {
197232
} else {
198233
// Attempt to delete, retrying on not empty errors because we may
199234
// need to wait some time for files to be removed from the filesystem.
200-
retry(|| delete(&dir, &[]), WinError::DIR_NOT_EMPTY)?;
235+
let name = string_buffer.set(&[]);
236+
retry(|| delete(&dir, name), WinError::DIR_NOT_EMPTY)?;
201237
restart = true;
202238
}
203239
}

0 commit comments

Comments
 (0)