fix(net): don't trust cmsg_len past the end of a truncated control buffer

src/net/send_recv/msg.rs

@@ -344,7 +344,7 @@ impl<'buf, 'slice, 'fd> SendAncillaryBuffer<'buf, 'slice, 'fd> {
         self.length = new_length;
 
         // Get the last header in the buffer.
-        let last_header = leap!(messages::Messages::new(buffer).last());
+        let (last_header, _) = leap!(messages::Messages::new(buffer).last());
 
         // Set the header fields.
         last_header.cmsg_len = unsafe { c::CMSG_LEN(source_len) } as _;
@@ -533,34 +533,48 @@ impl<'buf> AncillaryDrain<'buf> {
         }
     }
 
+    /// `space` is the number of bytes of buffer space at and after `msg`.
     fn advance(
         read_and_length: &mut Option<(&'buf mut usize, &'buf mut usize)>,
         msg: &c::cmsghdr,
+        space: usize,
     ) -> Option<RecvAncillaryMessage<'buf>> {
+        // Clamp the message length to the buffer. When `recvmsg` truncates
+        // control data to fit, Linux reduces `cmsg_len` to match what it
+        // wrote, but macOS leaves `cmsg_len` holding the untruncated length,
+        // so it can run past the end of the buffer.
+        let msg_len = (msg.cmsg_len as usize).min(space);
+
         // Advance the `read` pointer.
         if let Some((read, length)) = read_and_length {
-            let msg_len = msg.cmsg_len as usize;
             **read += msg_len;
             **length -= msg_len;
         }
 
-        Self::cvt_msg(msg)
+        Self::cvt_msg(msg, msg_len)
     }
 
     /// A closure that converts a message into a [`RecvAncillaryMessage`].
-    fn cvt_msg(msg: &c::cmsghdr) -> Option<RecvAncillaryMessage<'buf>> {
+    fn cvt_msg(msg: &c::cmsghdr, msg_len: usize) -> Option<RecvAncillaryMessage<'buf>> {
         unsafe {
-            // Get a pointer to the payload.
+            // Get a pointer to the payload. Use `msg_len` rather than
+            // `msg.cmsg_len`, as the message may have been truncated to fit
+            // in the buffer. If there isn't even a whole header, there's no
+            // message to report.
             let payload = c::CMSG_DATA(msg);
-            let payload_len = msg.cmsg_len as usize - c::CMSG_LEN(0) as usize;
-
-            // Get a mutable slice of the payload.
-            let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len);
+            let payload_len = msg_len.checked_sub(c::CMSG_LEN(0) as usize)?;
 
             // Determine what type it is.
             let (level, msg_type) = (msg.cmsg_level, msg.cmsg_type);
             match (level as _, msg_type as _) {
                 (c::SOL_SOCKET, c::SCM_RIGHTS) => {
+                    // Truncation can leave a partial file descriptor at the
+                    // end of the payload; round down to whole descriptors.
+                    let payload_len = payload_len - payload_len % size_of::<OwnedFd>();
+
+                    // Get a mutable slice of the payload.
+                    let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len);
+
                     // Create an iterator that reads out the file descriptors.
                     let fds = AncillaryIter::new(payload);
 
@@ -569,7 +583,7 @@ impl<'buf> AncillaryDrain<'buf> {
                 #[cfg(linux_kernel)]
                 (c::SOL_SOCKET, c::SCM_CREDENTIALS) => {
                     if payload_len >= size_of::<UCred>() {
-                        let ucred = payload.as_ptr().cast::<UCred>().read_unaligned();
+                        let ucred = payload.cast::<UCred>().read_unaligned();
                         Some(RecvAncillaryMessage::ScmCredentials(ucred))
                     } else {
                         None
@@ -586,7 +600,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
 
     fn next(&mut self) -> Option<Self::Item> {
         self.messages
-            .find_map(|ev| Self::advance(&mut self.read_and_length, ev))
+            .find_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
@@ -600,13 +614,13 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
         F: FnMut(B, Self::Item) -> B,
     {
         self.messages
-            .filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
+            .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
             .fold(init, f)
     }
 
     fn count(mut self) -> usize {
         self.messages
-            .filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
+            .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
             .count()
     }
 
@@ -615,7 +629,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
         Self: Sized,
     {
         self.messages
-            .filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
+            .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
             .last()
     }
 
@@ -624,7 +638,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
         Self: Sized,
     {
         self.messages
-            .filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
+            .filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
             .collect()
     }
 }
@@ -951,13 +965,21 @@ mod messages {
     }
 
     impl<'a> Iterator for Messages<'a> {
-        type Item = &'a mut c::cmsghdr;
+        /// A message header, along with the number of bytes of buffer space
+        /// at and after it, which is an upper bound on the size of the
+        /// message.
+        type Item = (&'a mut c::cmsghdr, usize);
 
         #[inline]
         fn next(&mut self) -> Option<Self::Item> {
             // Get the current header.
             let header = self.header?;
 
+            // Compute the number of bytes of buffer space at and after this
+            // header.
+            let end = (self.msghdr.msg_control as usize) + (self.msghdr.msg_controllen as usize);
+            let space = end.saturating_sub(header.as_ptr() as usize);
+
             // Get the next header.
             self.header = NonNull::new(unsafe { c::CMSG_NXTHDR(&self.msghdr, header.as_ptr()) });
 
@@ -967,7 +989,7 @@ mod messages {
             }
 
             // SAFETY: The lifetime of `header` is tied to this.
-            Some(unsafe { &mut *header.as_ptr() })
+            Some((unsafe { &mut *header.as_ptr() }, space))
         }
 
         fn size_hint(&self) -> (usize, Option<usize>) {

tests/net/cmsg.rs

@@ -33,3 +33,101 @@ fn test_buffer_sizes() {
     assert!(cmsg_space!(ScmRights(1)) * 2 >= cmsg_space!(ScmRights(1), ScmRights(1)));
     assert!(cmsg_space!(ScmRights(1), ScmRights(0)) >= cmsg_space!(ScmRights(1)));
 }
+
+/// Test that receiving more `SCM_RIGHTS` file descriptors than fit in the
+/// ancillary buffer doesn't panic and doesn't produce descriptors from
+/// outside the buffer.
+///
+/// Linux truncates the control data and adjusts `cmsg_len` to match, but
+/// macOS truncates the data and leaves `cmsg_len` holding the untruncated
+/// length, so the parsing code has to be prepared for a `cmsg_len` that runs
+/// past the end of the buffer.
+///
+/// Platforms report the truncation with `CTRUNC`, but not all of the
+/// environments rustix's CI runs in do, so this doesn't check for it.
+#[test]
+fn test_truncated_scm_rights() {
+    use rustix::fd::{AsFd, OwnedFd};
+    use rustix::io::{IoSlice, IoSliceMut};
+    use rustix::net::{
+        recvmsg, sendmsg, socket, socketpair, AddressFamily, RecvAncillaryBuffer,
+        RecvAncillaryMessage, RecvFlags, SendAncillaryBuffer, SendAncillaryMessage, SendFlags,
+        SocketFlags, SocketType,
+    };
+    use std::mem::MaybeUninit;
+
+    /// The number of file descriptors to send.
+    const NUM_FDS: usize = 16;
+    /// The number of file descriptors the receive buffer has room for.
+    const NUM_SLOTS: usize = 5;
+
+    crate::init();
+
+    let (send_sock, recv_sock) = socketpair(
+        AddressFamily::UNIX,
+        SocketType::STREAM,
+        SocketFlags::empty(),
+        None,
+    )
+    .unwrap();
+
+    // Make some file descriptors to send.
+    let fds: Vec<OwnedFd> = (0..NUM_FDS)
+        .map(|_| socket(AddressFamily::UNIX, SocketType::STREAM, None).unwrap())
+        .collect();
+    let borrowed: Vec<_> = fds.iter().map(AsFd::as_fd).collect();
+
+    let mut space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_FDS))];
+    let mut cmsg_buffer = SendAncillaryBuffer::new(space.as_mut_slice());
+    assert!(cmsg_buffer.push(SendAncillaryMessage::ScmRights(&borrowed)));
+
+    sendmsg(
+        &send_sock,
+        &[IoSlice::new(b"hello")],
+        &mut cmsg_buffer,
+        SendFlags::empty(),
+    )
+    .unwrap();
+
+    // Receive into a buffer with room for only `NUM_SLOTS` file descriptors.
+    let mut cmsg_space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_SLOTS))];
+    let mut cmsg_buffer = RecvAncillaryBuffer::new(cmsg_space.as_mut_slice());
+
+    let mut buffer = [0_u8; 5];
+    let result = recvmsg(
+        &recv_sock,
+        &mut [IoSliceMut::new(&mut buffer)],
+        &mut cmsg_buffer,
+        RecvFlags::empty(),
+    )
+    .unwrap();
+
+    assert_eq!(result.bytes, 5);
+    assert_eq!(&buffer, b"hello");
+
+    // Draining the buffer shouldn't panic.
+    let mut received = Vec::new();
+    for msg in cmsg_buffer.drain() {
+        match msg {
+            RecvAncillaryMessage::ScmRights(rights) => received.extend(rights),
+            _ => panic!("unexpected ancillary message"),
+        }
+    }
+
+    // Platforms deliver differing amounts of a truncated control message —
+    // Linux fills the buffer, FreeBSD delivers none of it — so don't assume
+    // how many descriptors come back. Do require that the message was
+    // truncated, and that every descriptor that did come back is one of the
+    // sockets that was sent, rather than something read from past the end of
+    // the buffer.
+    assert!(received.len() < NUM_FDS);
+    for fd in &received {
+        assert_eq!(
+            rustix::net::sockopt::socket_type(fd).unwrap(),
+            SocketType::STREAM
+        );
+    }
+
+    // Dropping the buffer drains it again; that shouldn't panic either.
+    drop(cmsg_buffer);
+}