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

memory corruption in socket::recvfrom() #1762

Closed
ensc opened this issue Jul 14, 2022 · 1 comment · Fixed by #1763
Closed

memory corruption in socket::recvfrom() #1762

ensc opened this issue Jul 14, 2022 · 1 comment · Fixed by #1763
Labels

Comments

@ensc
Copy link

ensc commented Jul 14, 2022

The addr field in

nix/src/sys/socket/mod.rs

Lines 1915 to 1924 in e5f354c

let mut addr = mem::MaybeUninit::uninit();
let mut len = mem::size_of::<T>() as socklen_t;
let ret = Errno::result(libc::recvfrom(
sockfd,
buf.as_ptr() as *mut c_void,
buf.len() as size_t,
0,
addr.as_mut_ptr() as *mut libc::sockaddr,
&mut len as *mut socklen_t))? as usize;
is allocated only with a size of libc::sockaddr. This will overflow for most sockaddr types.

I suggest to write this as

diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs
index 6386e62..59b274a 100644
--- a/src/sys/socket/mod.rs
+++ b/src/sys/socket/mod.rs
@@ -1912,7 +1912,7 @@ pub fn recvfrom<T:SockaddrLike>(sockfd: RawFd, buf: &mut [u8])
     -> Result<(usize, Option<T>)>
 {
     unsafe {
-        let mut addr = mem::MaybeUninit::uninit();
+        let mut addr = mem::MaybeUninit::<T>::uninit();
         let mut len = mem::size_of::<T>() as socklen_t;
 
         let ret = Errno::result(libc::recvfrom(
@@ -1923,7 +1923,7 @@ pub fn recvfrom<T:SockaddrLike>(sockfd: RawFd, buf: &mut [u8])
             addr.as_mut_ptr() as *mut libc::sockaddr,
             &mut len as *mut socklen_t))? as usize;
 
-        Ok((ret, T::from_raw(&addr.assume_init(), Some(len))))
+        Ok((ret, T::from_raw(addr.assume_init().as_ptr(), Some(len))))
     }
 }
 
@asomers
Copy link
Member

asomers commented Jul 14, 2022

Oh, good catch! I'll fix it shortly.

@asomers asomers added the A-bug label Jul 14, 2022
asomers added a commit to asomers/nix that referenced this issue Jul 14, 2022
IPv4 and stream sockets are unaffected, but for datagram sockets of
other address types libc::recvfrom might overwrite part of the stack.

Fixes nix-rust#1762
bors bot added a commit that referenced this issue Jul 14, 2022
1763: Fix a buffer overflow in sys::socket::recvfrom r=posborne a=asomers

IPv4 and stream sockets are unaffected, but for datagram sockets of
other address types libc::recvfrom might overwrite part of the stack.

Fixes #1762

Co-authored-by: Alan Somers <[email protected]>
@bors bors bot closed this as completed in e0e768e Jul 14, 2022
rtzoeller pushed a commit to rtzoeller/nix that referenced this issue Jul 17, 2022
IPv4 and stream sockets are unaffected, but for datagram sockets of
other address types libc::recvfrom might overwrite part of the stack.

Fixes nix-rust#1762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants