-
Notifications
You must be signed in to change notification settings - Fork 719
Description
Thank you for your outstanding work!
I have found a soundness bug in the recvfrom wrapper that leads to Undefined Behavior (UB).
The wrapper incorrectly calls addr.assume_init() on the MaybeUninit buffer (addr) after it's passed to the libc::recvfrom function. This assertion is invalid because the POSIX specification for recvfrom makes no guarantee that the entire buffer addr will be initialized.
pub fn recvfrom<T: SockaddrLike>(
sockfd: RawFd,
buf: &mut [u8],
) -> Result<(usize, Option<T>)> {
unsafe {
let mut addr = mem::MaybeUninit::<T>::uninit();
let mut len = mem::size_of_val(&addr) as socklen_t;
let ret = Errno::result(libc::recvfrom(
sockfd,
buf.as_mut_ptr().cast(),
buf.len() as size_t,
0,
addr.as_mut_ptr().cast(),
&mut len as *mut socklen_t,
))? as usize;
// 🚨 UNDEFINED BEHAVIOR IS HERE:
// `addr` is not guaranteed to be fully initialized.
Ok((ret, T::from_raw(addr.assume_init().as_ptr(), Some(len))))
}
}Calling MaybeUninit::assume_init() makes an absolute promise to the compiler that all bytes of the value are now valid and initialized. However, libc::recvfrom can (and often does) violate this promise in at least two common scenarios:
-
T is likely a type like sockaddr_storage to be ableg to hold any address type. If recvfrom receives a datagram from an IPv4 source, it will write a sockaddr_in (16 bytes) into the addr buffer. It will correctly update len to 16. The addr buffer is now only partially initialized. The bytes from 16 to size_of() are still uninitialized. Calling addr.assume_init() creates a T instance from this partially-initialized memory, which is UB.
-
The recvfrom POSIX documentation states: "If the address argument is not a null pointer and the protocol does not provide the source address of messages, the value stored in the object pointed to by address is unspecified." If this function is called on a SOCK_STREAM (TCP) socket, libc::recvfrom may not write any bytes to addr. In this case, addr remains completely uninitialized. Calling addr.assume_init() is immediate UB.