Skip to content
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

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

Merged
merged 4 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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: (),
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
}

/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `PathBuffer` field!
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
#[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
69 changes: 41 additions & 28 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, Align8};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::thread;

Expand Down Expand Up @@ -326,9 +326,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 = Align8([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,9 +389,9 @@ 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 = Align8([0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) {
attr.reparse_tag = buf.ReparseTag;
attr.reparse_tag = (*buf).ReparseTag;
}
}
Ok(attr)
Expand Down Expand Up @@ -458,48 +458,57 @@ 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 Align8<[u8]>,
) -> io::Result<(c::DWORD, *const c::REPARSE_DATA_BUFFER)> {
unsafe {
let mut bytes = 0;
cvt({
// Grab this in advance to avoid it invalidating the pointer
// we get from `space.0.as_mut_ptr()`.
let len = space.0.len();
c::DeviceIoControl(
self.handle.as_raw_handle(),
c::FSCTL_GET_REPARSE_POINT,
ptr::null_mut(),
0,
space.as_mut_ptr() as *mut _,
space.len() as c::DWORD,
space.0.as_mut_ptr().cast(),
len as c::DWORD,
&mut bytes,
ptr::null_mut(),
)
})?;
Ok((bytes, &*(space.as_ptr() as *const c::REPARSE_DATA_BUFFER)))
const _: () = assert!(core::mem::align_of::<c::REPARSE_DATA_BUFFER>() <= 8);
Ok((bytes, space.0.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 = Align8([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 Expand Up @@ -649,18 +658,18 @@ impl File {

/// A buffer for holding directory entries.
struct DirBuff {
buffer: Vec<u8>,
buffer: Box<Align8<[u8; Self::BUFFER_SIZE]>>,
}
impl DirBuff {
const BUFFER_SIZE: usize = 1024;
fn new() -> Self {
const BUFFER_SIZE: usize = 1024;
Self { buffer: vec![0_u8; BUFFER_SIZE] }
Self { buffer: Box::new(Align8([0u8; Self::BUFFER_SIZE])) }
}
fn capacity(&self) -> usize {
self.buffer.len()
self.buffer.0.len()
}
fn as_mut_ptr(&mut self) -> *mut u8 {
self.buffer.as_mut_ptr().cast()
self.buffer.0.as_mut_ptr().cast()
}
/// Returns a `DirBuffIter`.
fn iter(&self) -> DirBuffIter<'_> {
Expand All @@ -669,7 +678,7 @@ impl DirBuff {
}
impl AsRef<[u8]> for DirBuff {
fn as_ref(&self) -> &[u8] {
&self.buffer
&self.buffer.0
}
}

Expand Down Expand Up @@ -697,9 +706,12 @@ impl<'a> Iterator for DirBuffIter<'a> {
// used to get the file name slice.
let (name, is_directory, next_entry) = unsafe {
let info = buffer.as_ptr().cast::<c::FILE_ID_BOTH_DIR_INFO>();
// Guaranteed to be aligned in documentation for
// https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_both_dir_info
assert!(info.is_aligned());
let next_entry = (*info).NextEntryOffset as usize;
let name = crate::slice::from_raw_parts(
(*info).FileName.as_ptr().cast::<u16>(),
ptr::addr_of!((*info).FileName).cast::<u16>(),
(*info).FileNameLength as usize / size_of::<u16>(),
);
let is_directory = ((*info).FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) != 0;
Expand Down Expand Up @@ -1337,9 +1349,10 @@ fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
let h = f.as_inner().as_raw_handle();

unsafe {
let mut data = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let db = data.as_mut_ptr() as *mut c::REPARSE_MOUNTPOINT_DATA_BUFFER;
let buf = &mut (*db).ReparseTarget as *mut c::WCHAR;
let mut data = Align8([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
let data_ptr = data.0.as_mut_ptr();
let db = data_ptr.cast::<c::REPARSE_MOUNTPOINT_DATA_BUFFER>();
let buf = ptr::addr_of_mut!((*db).ReparseTarget).cast::<c::WCHAR>();
let mut i = 0;
// FIXME: this conversion is very hacky
let v = br"\??\";
Expand All @@ -1359,7 +1372,7 @@ fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
cvt(c::DeviceIoControl(
h as *mut _,
c::FSCTL_SET_REPARSE_POINT,
data.as_ptr() as *mut _,
data_ptr.cast(),
(*db).ReparseDataLength + 8,
ptr::null_mut(),
0,
Expand Down
8 changes: 8 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,11 @@ pub fn abort_internal() -> ! {
}
crate::intrinsics::abort();
}

/// Align the inner value to 8 bytes.
///
/// This is enough for almost all of the buffers we're likely to work with in
/// the Windows APIs we use.
#[repr(C, align(8))]
#[derive(Copy, Clone)]
pub(crate) struct Align8<T: ?Sized>(pub T);