Hi,
I found a memory-safety/soundness issue in this crate while scanning Rust code for potential vulnerabilities. This PR contains a fix for the issue.
Issue Description
|
let mut overlapped: Box<OVERLAPPED> = Box::new(mem::zeroed()); |
|
// When using callback based async requests, we are allowed to use the hEvent member |
|
// for our own purposes |
|
|
|
let req_buf = request.buffer.as_mut_ptr() as *mut c_void; |
|
let request_p = Box::into_raw(request) as isize; |
|
overlapped.hEvent = request_p; |
|
|
|
// This is using an asynchronous call with a completion routine for receiving notifications |
|
// An I/O completion port would probably be more performant |
|
let ret = ReadDirectoryChangesW( |
|
handle, |
|
req_buf, |
|
BUF_SIZE, |
|
monitor_subdir, |
|
flags, |
|
&mut 0u32 as *mut u32, // not used for async reqs |
|
&mut *overlapped as *mut OVERLAPPED, |
|
Some(handle_event), |
|
); |
|
|
|
if ret == 0 { |
|
// error reading. retransmute request memory to allow drop. |
|
// allow overlapped to drop by omitting forget() |
|
let request: Box<ReadDirectoryRequest> = mem::transmute(request_p); |
|
|
|
ReleaseSemaphore(request.data.complete_sem, 1, ptr::null_mut()); |
|
} else { |
|
// read ok. forget overlapped to let the completion routine handle memory |
|
mem::forget(overlapped); |
If a panic!() occurs between the Box::new() function and std::mem::forget, a double free vulnerability emerges.
Fix
In Rust, std::mem::forget does not actually free the memory, instead it simply allows the memory to leak. This can lead to double free when the data object goes out of scope and its destructor is called automatically. The modification here uses std::mem::ManuallyDrop to wrap data. This ensures that data will not be automatically dropped when it goes out of scope, thus avoiding a double free scenario. With ManuallyDrop, we explicitly state that the data variable should not be dropped, thus avoiding any potential double free issues.