From b5ef2871a9db5936a6ba3b8b2a3124f8039ce6ad Mon Sep 17 00:00:00 2001 From: Shardul Natu Date: Fri, 23 Oct 2026 09:46:50 -0700 Subject: [PATCH] fix(dua-core): avoid reading past HEADER_SIZE in Windows ReadDir::entry_at `FILE_ID_BOTH_DIR_INFO` has a 104-byte fixed header (`HEADER_SIZE`) followed by `FileName: [u16; 1]` (2 bytes) and 6 bytes of trailing alignment padding (`size_of::() == 112`). `ReadDir::entry_at` validated `offset <= buffer_len.saturating_sub(HEADER_SIZE)` and then called `info_ptr.read_unaligned()`, which reads 112 bytes and can read up to 8 bytes past the end of `self.buffer` when a valid entry with a 1-3 UTF-16 code unit filename (106-110 bytes) sits at the end of the buffer. Copy only `HEADER_SIZE` bytes into `FILE_ID_BOTH_DIR_INFO::default()` via `std::ptr::copy_nonoverlapping`, compute `name_ptr` directly from `offset + HEADER_SIZE`, and use `checked_add(HEADER_SIZE)` instead of `saturating_sub(HEADER_SIZE)` so buffers smaller than `HEADER_SIZE` are also rejected. --- crates/dua-lib/src/windows.rs | 83 +++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 13 deletions(-) diff --git a/crates/dua-lib/src/windows.rs b/crates/dua-lib/src/windows.rs index 8d1b7b7d..4c147eba 100644 --- a/crates/dua-lib/src/windows.rs +++ b/crates/dua-lib/src/windows.rs @@ -266,22 +266,26 @@ impl ReadDir { fn entry_at(&self, offset: usize) -> io::Result<(Entry, Option)> { const HEADER_SIZE: usize = std::mem::offset_of!(FILE_ID_BOTH_DIR_INFO, FileName); let buffer_len = self.buffer.len() * size_of::(); - if offset > buffer_len.saturating_sub(HEADER_SIZE) { + if offset + .checked_add(HEADER_SIZE) + .is_none_or(|end| end > buffer_len) + { return Err(io::Error::new( io::ErrorKind::InvalidData, "Windows directory record header exceeds its buffer", )); } - // SAFETY: validation above proves the record header lies within the aligned backing - // allocation. Individual fields are read unaligned because record offsets need not be. - let info_ptr = unsafe { - self.buffer - .as_ptr() - .byte_add(offset) - .cast::() - }; - // SAFETY: `info_ptr` addresses a complete in-bounds header validated above. - let info = unsafe { info_ptr.read_unaligned() }; + let mut info = FILE_ID_BOTH_DIR_INFO::default(); + // SAFETY: `offset + HEADER_SIZE <= buffer_len` was validated above, and `HEADER_SIZE` + // is the byte offset of `FileName` within `FILE_ID_BOTH_DIR_INFO`. Copying `HEADER_SIZE` + // bytes as `u8` requires no alignment and stays in-bounds of both `self.buffer` and `info`. + unsafe { + std::ptr::copy_nonoverlapping( + self.buffer.as_ptr().cast::().add(offset), + (&raw mut info).cast::(), + HEADER_SIZE, + ); + } if info.FileNameLength % 2 != 0 { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -304,7 +308,12 @@ impl ReadDir { } // SAFETY: the filename range was validated against the buffer above. It may be unaligned, // so each UTF-16 code unit is copied with `read_unaligned` below. - let name_ptr = unsafe { (&raw const (*info_ptr).FileName).cast::() }; + let name_ptr = unsafe { + self.buffer + .as_ptr() + .byte_add(offset + HEADER_SIZE) + .cast::() + }; let name = (0..name_len) // SAFETY: `idx` stays within the validated filename range. .map(|idx| unsafe { name_ptr.add(idx).read_unaligned() }) @@ -315,7 +324,10 @@ impl ReadDir { } else { let next = offset .checked_add(info.NextEntryOffset as usize) - .filter(|next| *next <= buffer_len.saturating_sub(HEADER_SIZE)) + .filter(|next| { + next.checked_add(HEADER_SIZE) + .is_some_and(|end| end <= buffer_len) + }) .ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidData, @@ -781,4 +793,49 @@ mod tests { OsString::from(r"\\?\UNC\server\share\dir") ); } + + #[test] + fn entry_at_reads_short_filename_at_end_of_buffer_without_overreading_padding() { + const HEADER_SIZE: usize = std::mem::offset_of!(FILE_ID_BOTH_DIR_INFO, FileName); + let dir = tempfile::tempdir().unwrap(); + let mut read_dir = + ReadDir::open(Arc::from(dir.path()), 1, crate::Options::default()).unwrap(); + + // 14 u64 words = 112 bytes. Place a 106-byte record (104-byte header + 1 UTF-16 code unit) + // at offset 6 so only 106 bytes remain in the buffer (< size_of::() == 112). + read_dir.buffer = vec![0; 14]; + let offset = 112 - (HEADER_SIZE + size_of::()); + let header = FILE_ID_BOTH_DIR_INFO { + FileNameLength: 2, + EndOfFile: 42, + AllocationSize: 64, + FileId: 7, + ..FILE_ID_BOTH_DIR_INFO::default() + }; + let name_unit = u16::from(b'a'); + // SAFETY: `offset + HEADER_SIZE + size_of::() == 112`, matching the 112-byte buffer. + unsafe { + let dst = read_dir.buffer.as_mut_ptr().cast::().add(offset); + std::ptr::copy_nonoverlapping((&raw const header).cast::(), dst, HEADER_SIZE); + std::ptr::copy_nonoverlapping( + (&raw const name_unit).cast::(), + dst.add(HEADER_SIZE), + size_of::(), + ); + } + + let (entry, next_offset) = read_dir.entry_at(offset).unwrap(); + assert_eq!(entry.file_name, OsString::from("a")); + assert_eq!(next_offset, None); + let metadata = entry.metadata.unwrap().unwrap(); + assert_eq!(metadata.len(), 42); + assert_eq!(metadata.allocated_size(), 64); + + // A buffer smaller than HEADER_SIZE must fail cleanly rather than saturating to 0. + read_dir.buffer = vec![0; 1]; + assert_eq!( + read_dir.entry_at(0).err().unwrap().kind(), + io::ErrorKind::InvalidData + ); + } }