Skip to content

Commit a405950

Browse files
authored
Merge pull request #737 from workingjubilee/address-dbghelp-soundness-risks
Remove padding bytes risk in dbghelp with MaybeUninit
2 parents dccdb4d + 06dca9a commit a405950

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

src/symbolize/dbghelp.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@
1919

2020
use super::super::{dbghelp, windows_sys::*};
2121
use super::{BytesOrWideString, ResolveWhat, SymbolName};
22+
use core::cmp;
2223
use core::ffi::c_void;
2324
use core::marker;
24-
use core::mem;
25+
use core::mem::{self, MaybeUninit};
2526
use core::ptr;
2627
use core::slice;
2728

@@ -217,19 +218,23 @@ unsafe fn resolve_with_inline(
217218
Some(())
218219
}
219220

221+
/// This function is only meant to be called with certain Windows API functions as its arguments,
222+
/// using closures to simplify away here-unspecified arguments:
223+
/// - `sym_from_addr`: either `SymFromAddrW` or `SymFromInlineContextW`
224+
/// - `get_line_from_addr`: `SymGetLineFromAddrW64` or `SymGetLineFromInlineContextW`
220225
unsafe fn do_resolve(
221226
sym_from_addr: impl FnOnce(*mut SYMBOL_INFOW) -> BOOL,
222227
get_line_from_addr: impl FnOnce(&mut IMAGEHLP_LINEW64) -> BOOL,
223228
cb: &mut dyn FnMut(&super::Symbol),
224229
) {
225230
const SIZE: usize = 2 * MAX_SYM_NAME as usize + mem::size_of::<SYMBOL_INFOW>();
226-
let mut data = Aligned8([0u8; SIZE]);
227-
let info = unsafe { &mut *data.0.as_mut_ptr().cast::<SYMBOL_INFOW>() };
228-
info.MaxNameLen = MAX_SYM_NAME as u32;
231+
let mut data = MaybeUninit::<Aligned8<[u8; SIZE]>>::zeroed();
232+
let info = data.as_mut_ptr().cast::<SYMBOL_INFOW>();
233+
unsafe { (*info).MaxNameLen = MAX_SYM_NAME as u32 };
229234
// the struct size in C. the value is different to
230235
// `size_of::<SYMBOL_INFOW>() - MAX_SYM_NAME + 1` (== 81)
231236
// due to struct alignment.
232-
info.SizeOfStruct = 88;
237+
unsafe { (*info).SizeOfStruct = 88 };
233238

234239
if sym_from_addr(info) != TRUE {
235240
return;
@@ -238,8 +243,10 @@ unsafe fn do_resolve(
238243
// If the symbol name is greater than MaxNameLen, SymFromAddrW will
239244
// give a buffer of (MaxNameLen - 1) characters and set NameLen to
240245
// the real value.
241-
let name_len = ::core::cmp::min(info.NameLen as usize, info.MaxNameLen as usize - 1);
242-
let name_ptr = info.Name.as_ptr().cast::<u16>();
246+
// SAFETY: We assume NameLen has been initialized by SymFromAddrW, and we initialized MaxNameLen
247+
let name_len = unsafe { cmp::min((*info).NameLen as usize, (*info).MaxNameLen as usize - 1) };
248+
// Name must be initialized by SymFromAddrW, but we only interact with it as a pointer anyways.
249+
let name_ptr = (&raw const (*info).Name).cast::<u16>();
243250

244251
// Reencode the utf-16 symbol to utf-8 so we can use `SymbolName::new` like
245252
// all other platforms
@@ -296,7 +303,8 @@ unsafe fn do_resolve(
296303
cb(&super::Symbol {
297304
inner: Symbol {
298305
name,
299-
addr: info.Address as *mut _,
306+
// SAFETY: we assume Address has been initialized by SymFromAddrW
307+
addr: unsafe { (*info).Address } as *mut _,
300308
line: lineno,
301309
filename,
302310
_filename_cache: unsafe { cache(filename) },

0 commit comments

Comments
 (0)