From 3313cd6f7fae3ad4f4673ae88fb38c0faad2a8b9 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 19 Jun 2026 18:53:44 +0100 Subject: [PATCH] std: fix stack buffer overflow in Windows junction_point The guard checked `data_len > u16::MAX`, allowing paths far larger than `PathBuffer` (a fixed 16384-element array), which the subsequent single `copy_from` then overflows. Bound against `MAXIMUM_REPARSE_DATA_BUFFER_SIZE` plus header instead, matching the kernel's limit. --- library/std/src/fs/tests.rs | 19 ++++++++++++++++ library/std/src/sys/fs/windows.rs | 37 +++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 4f2fa7fbc591e..c3fdb2bf741e3 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1723,6 +1723,25 @@ fn create_dir_all_with_junctions() { assert!(d.exists()); } +#[test] +#[cfg(windows)] +fn junction_point_overlong_path() { + // Regression test: an `original` path long enough to exceed the inline + // reparse buffer used to be copied past the end of the stack array. It must + // now be rejected with a clean error instead of overflowing. + let tmpdir = tmpdir(); + let link = tmpdir.join("junction"); + + // The `\\?\` prefix bypasses MAX_PATH normalization so the path is copied + // through verbatim. 20_000 code units lands in the old overflow window: it + // passed the previous `> u16::MAX` byte check yet exceeded the buffer. + let mut original = String::from(r"\\?\C:\"); + original.push_str(&"a".repeat(20_000)); + + let err = junction_point(Path::new(&original), &link).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); +} + #[test] fn metadata_access_times() { let start_time = SystemTime::now(); diff --git a/library/std/src/sys/fs/windows.rs b/library/std/src/sys/fs/windows.rs index e3e7b081b47d5..67cd8e8612344 100644 --- a/library/std/src/sys/fs/windows.rs +++ b/library/std/src/sys/fs/windows.rs @@ -1676,33 +1676,46 @@ pub fn junction_point(original: &Path, link: &Path) -> io::Result<()> { SubstituteNameLength: u16, PrintNameOffset: u16, PrintNameLength: u16, - PathBuffer: [MaybeUninit; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize], + // `MAXIMUM_REPARSE_DATA_BUFFER_SIZE` is a size in bytes, but this is a + // buffer of `u16`s, so it holds half as many elements. + PathBuffer: [MaybeUninit; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize / 2], } - let data_len = 12 + (abs_path.len() * 2); - if data_len > u16::MAX as usize { - return Err(io::const_error!(io::ErrorKind::InvalidInput, "`original` path is too long")); - } - let data_len = data_len as u16; let mut header = MountPointBuffer { ReparseTag: c::IO_REPARSE_TAG_MOUNT_POINT, - ReparseDataLength: data_len, + ReparseDataLength: 0, // filled in below Reserved: 0, SubstituteNameOffset: 0, SubstituteNameLength: (abs_path.len() * 2) as u16, + // The print name follows the substitute name and its null terminator. PrintNameOffset: ((abs_path.len() + 1) * 2) as u16, PrintNameLength: 0, - PathBuffer: [MaybeUninit::uninit(); c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize], + PathBuffer: [MaybeUninit::uninit(); c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize / 2], }; + // A mount point reparse point requires both the substitute name and the + // (empty) print name to be null terminated, even though their lengths are + // explicit. Bounds-check and copy in a single step so an over-long path + // fails cleanly instead of overflowing the buffer. + let Some(path_buffer) = header.PathBuffer.get_mut(..abs_path.len() + 2) else { + return Err(io::const_error!(io::ErrorKind::InvalidInput, "`original` path is too long")); + }; + let (substitute_name, terminators) = path_buffer.split_at_mut(abs_path.len()); + substitute_name.write_copy_of_slice(&abs_path); + terminators.write_copy_of_slice(&[0, 0]); + // Total size of the structure: the fixed header fields, the path, and the + // two null terminators. + let total_len = offset_of!(MountPointBuffer, PathBuffer) + (abs_path.len() + 2) * 2; + // `ReparseDataLength` counts only the bytes after the 8-byte common header + // (`ReparseTag`, `ReparseDataLength`, `Reserved`), i.e. + // `SubstituteNameLength + PrintNameLength + 12`. + header.ReparseDataLength = + (total_len - offset_of!(MountPointBuffer, SubstituteNameOffset)) as u16; unsafe { - let ptr = header.PathBuffer.as_mut_ptr(); - ptr.copy_from(abs_path.as_ptr().cast_uninit(), abs_path.len()); - let mut ret = 0; cvt(c::DeviceIoControl( d.as_raw_handle(), c::FSCTL_SET_REPARSE_POINT, (&raw const header).cast::(), - data_len as u32 + 8, + total_len as u32, ptr::null_mut(), 0, &mut ret,