Skip to content

Fix UB from misalignment and provenance widening in std::sys::windows #101171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ pub struct FILE_END_OF_FILE_INFO {
pub EndOfFile: LARGE_INTEGER,
}

/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `rest` field!
#[repr(C)]
pub struct REPARSE_DATA_BUFFER {
pub ReparseTag: c_uint,
Expand All @@ -511,6 +513,8 @@ pub struct REPARSE_DATA_BUFFER {
pub rest: (),
}

/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `PathBuffer` field!
#[repr(C)]
pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort,
Expand All @@ -521,6 +525,8 @@ pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
pub PathBuffer: WCHAR,
}

/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `PathBuffer` field!
#[repr(C)]
pub struct MOUNT_POINT_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort,
Expand Down
40 changes: 24 additions & 16 deletions library/std/src/sys/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::slice;
use crate::sync::Arc;
use crate::sys::handle::Handle;
use crate::sys::time::SystemTime;
use crate::sys::{c, cvt};
use crate::sys::{c, cvt, AlignedAs};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::thread;

Expand Down Expand Up @@ -47,6 +47,9 @@ pub struct ReadDir {
first: Option<c::WIN32_FIND_DATAW>,
}

type AlignedReparseBuf =
AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>;

struct FindNextFileHandle(c::HANDLE);

unsafe impl Send for FindNextFileHandle {}
Expand Down Expand Up @@ -326,9 +329,9 @@ impl File {
cvt(c::GetFileInformationByHandle(self.handle.as_raw_handle(), &mut info))?;
let mut reparse_tag = 0;
if info.dwFileAttributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0 {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut b = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) {
reparse_tag = buf.ReparseTag;
reparse_tag = (*buf).ReparseTag;
}
}
Ok(FileAttr {
Expand Down Expand Up @@ -389,7 +392,7 @@ impl File {
attr.file_size = info.AllocationSize as u64;
attr.number_of_links = Some(info.NumberOfLinks);
if attr.file_type().is_reparse_point() {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut b = AlignedReparseBuf::new([0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) {
attr.reparse_tag = buf.ReparseTag;
}
Expand Down Expand Up @@ -458,10 +461,13 @@ impl File {
Ok(Self { handle: self.handle.try_clone()? })
}

fn reparse_point<'a>(
// NB: returned pointer is derived from `space`, and has provenance to
// match. A raw pointer is returned rather than a reference in order to
// avoid narrowing provenance to the actual `REPARSE_DATA_BUFFER`.
fn reparse_point(
&self,
space: &'a mut [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE],
) -> io::Result<(c::DWORD, &'a c::REPARSE_DATA_BUFFER)> {
space: &mut AlignedReparseBuf,
) -> io::Result<(c::DWORD, *const c::REPARSE_DATA_BUFFER)> {
unsafe {
let mut bytes = 0;
cvt({
Expand All @@ -470,36 +476,38 @@ impl File {
c::FSCTL_GET_REPARSE_POINT,
ptr::null_mut(),
0,
space.as_mut_ptr() as *mut _,
space.len() as c::DWORD,
space.value.as_mut_ptr() as *mut _,
space.value.len() as c::DWORD,
&mut bytes,
ptr::null_mut(),
)
})?;
Ok((bytes, &*(space.as_ptr() as *const c::REPARSE_DATA_BUFFER)))
Ok((bytes, space.value.as_ptr().cast::<c::REPARSE_DATA_BUFFER>()))
}
}

fn readlink(&self) -> io::Result<PathBuf> {
let mut space = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut space = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
let (_bytes, buf) = self.reparse_point(&mut space)?;
unsafe {
let (path_buffer, subst_off, subst_len, relative) = match buf.ReparseTag {
let (path_buffer, subst_off, subst_len, relative) = match (*buf).ReparseTag {
c::IO_REPARSE_TAG_SYMLINK => {
let info: *const c::SYMBOLIC_LINK_REPARSE_BUFFER =
&buf.rest as *const _ as *const _;
ptr::addr_of!((*buf).rest).cast();
assert!(info.is_aligned());
(
&(*info).PathBuffer as *const _ as *const u16,
ptr::addr_of!((*info).PathBuffer).cast::<u16>(),
(*info).SubstituteNameOffset / 2,
(*info).SubstituteNameLength / 2,
(*info).Flags & c::SYMLINK_FLAG_RELATIVE != 0,
)
}
c::IO_REPARSE_TAG_MOUNT_POINT => {
let info: *const c::MOUNT_POINT_REPARSE_BUFFER =
&buf.rest as *const _ as *const _;
ptr::addr_of!((*buf).rest).cast();
assert!(info.is_aligned());
(
&(*info).PathBuffer as *const _ as *const u16,
ptr::addr_of!((*info).PathBuffer).cast::<u16>(),
(*info).SubstituteNameOffset / 2,
(*info).SubstituteNameLength / 2,
false,
Expand Down
23 changes: 23 additions & 0 deletions library/std/src/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,26 @@ pub fn abort_internal() -> ! {
}
crate::intrinsics::abort();
}

/// Used for some win32 buffers which are stack allocated, for example:
/// `AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>`
#[repr(C)]
#[derive(Copy, Clone)]
pub struct AlignedAs<Aligner, Alignee: ?Sized> {
/// Use `[Aligner; 0]` as a sort of `PhantomAlignNextField<Aligner>`. This
/// is a bit of a hack, and could break (in a way that's caught by tests) if
/// #81996 is fixed.
aligner: [Aligner; 0],
/// The aligned value. Public rather than exposed via accessors so that if
/// needed it can be used with `addr_of` and such (also, this is less code).
pub value: Alignee,
}

impl<Aligner, Alignee> AlignedAs<Aligner, Alignee> {
// This is frequently used with large stack buffers, so force-inline to
// try and avoid using 2x as much stack space in debug builds.
#[inline(always)]
pub const fn new(value: Alignee) -> Self {
Self { aligner: [], value }
}
}
18 changes: 18 additions & 0 deletions library/std/src/sys/windows/os/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,21 @@ fn ntstatus_error() {
.contains("FormatMessageW() returned error")
);
}

#[test]
fn smoketest_aligned_as() {
use crate::{
mem::{align_of, size_of},
ptr::addr_of,
sys::{c, AlignedAs},
};
type AlignedReparseBuf =
AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>;
assert!(size_of::<AlignedReparseBuf>() >= c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
assert_eq!(align_of::<AlignedReparseBuf>(), align_of::<c::REPARSE_DATA_BUFFER>());
let a = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
// Quick and dirty offsetof check.
assert_eq!(addr_of!(a).cast::<u8>(), addr_of!(a.value).cast::<u8>());
// Smoke check that it's actually aligned.
assert!(addr_of!(a.value).is_aligned_to(align_of::<c::REPARSE_DATA_BUFFER>()));
}