Skip to content

Commit 6a81010

Browse files
xrlclaude
andcommitted
fix(net): don't trust cmsg_len past the end of a truncated control buffer
On macOS, `recvmsg` truncates control data to fit the buffer but leaves `cmsg_len` untruncated, so `AncillaryDrain::advance` underflowed the remaining length and the drain in `Drop` panicked again, aborting the process; with overflow checks off, `cvt_msg` instead sliced past the buffer and yielded `OwnedFd`s read from uninitialized memory. `Messages` now yields the buffer space at each header, `advance` clamps `cmsg_len` to it, and `cvt_msg` rounds `SCM_RIGHTS` payloads down to whole descriptors. Fixes #1683. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 287214b commit 6a81010

2 files changed

Lines changed: 137 additions & 17 deletions

File tree

src/net/send_recv/msg.rs

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ impl<'buf, 'slice, 'fd> SendAncillaryBuffer<'buf, 'slice, 'fd> {
344344
self.length = new_length;
345345

346346
// Get the last header in the buffer.
347-
let last_header = leap!(messages::Messages::new(buffer).last());
347+
let (last_header, _) = leap!(messages::Messages::new(buffer).last());
348348

349349
// Set the header fields.
350350
last_header.cmsg_len = unsafe { c::CMSG_LEN(source_len) } as _;
@@ -533,34 +533,48 @@ impl<'buf> AncillaryDrain<'buf> {
533533
}
534534
}
535535

536+
/// `space` is the number of bytes of buffer space at and after `msg`.
536537
fn advance(
537538
read_and_length: &mut Option<(&'buf mut usize, &'buf mut usize)>,
538539
msg: &c::cmsghdr,
540+
space: usize,
539541
) -> Option<RecvAncillaryMessage<'buf>> {
542+
// Clamp the message length to the buffer. When `recvmsg` truncates
543+
// control data to fit, Linux reduces `cmsg_len` to match what it
544+
// wrote, but macOS leaves `cmsg_len` holding the untruncated length,
545+
// so it can run past the end of the buffer.
546+
let msg_len = (msg.cmsg_len as usize).min(space);
547+
540548
// Advance the `read` pointer.
541549
if let Some((read, length)) = read_and_length {
542-
let msg_len = msg.cmsg_len as usize;
543550
**read += msg_len;
544551
**length -= msg_len;
545552
}
546553

547-
Self::cvt_msg(msg)
554+
Self::cvt_msg(msg, msg_len)
548555
}
549556

550557
/// A closure that converts a message into a [`RecvAncillaryMessage`].
551-
fn cvt_msg(msg: &c::cmsghdr) -> Option<RecvAncillaryMessage<'buf>> {
558+
fn cvt_msg(msg: &c::cmsghdr, msg_len: usize) -> Option<RecvAncillaryMessage<'buf>> {
552559
unsafe {
553-
// Get a pointer to the payload.
560+
// Get a pointer to the payload. Use `msg_len` rather than
561+
// `msg.cmsg_len`, as the message may have been truncated to fit
562+
// in the buffer. If there isn't even a whole header, there's no
563+
// message to report.
554564
let payload = c::CMSG_DATA(msg);
555-
let payload_len = msg.cmsg_len as usize - c::CMSG_LEN(0) as usize;
556-
557-
// Get a mutable slice of the payload.
558-
let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len);
565+
let payload_len = msg_len.checked_sub(c::CMSG_LEN(0) as usize)?;
559566

560567
// Determine what type it is.
561568
let (level, msg_type) = (msg.cmsg_level, msg.cmsg_type);
562569
match (level as _, msg_type as _) {
563570
(c::SOL_SOCKET, c::SCM_RIGHTS) => {
571+
// Truncation can leave a partial file descriptor at the
572+
// end of the payload; round down to whole descriptors.
573+
let payload_len = payload_len - payload_len % size_of::<OwnedFd>();
574+
575+
// Get a mutable slice of the payload.
576+
let payload: &'buf mut [u8] = slice::from_raw_parts_mut(payload, payload_len);
577+
564578
// Create an iterator that reads out the file descriptors.
565579
let fds = AncillaryIter::new(payload);
566580

@@ -569,7 +583,7 @@ impl<'buf> AncillaryDrain<'buf> {
569583
#[cfg(linux_kernel)]
570584
(c::SOL_SOCKET, c::SCM_CREDENTIALS) => {
571585
if payload_len >= size_of::<UCred>() {
572-
let ucred = payload.as_ptr().cast::<UCred>().read_unaligned();
586+
let ucred = payload.cast::<UCred>().read_unaligned();
573587
Some(RecvAncillaryMessage::ScmCredentials(ucred))
574588
} else {
575589
None
@@ -586,7 +600,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
586600

587601
fn next(&mut self) -> Option<Self::Item> {
588602
self.messages
589-
.find_map(|ev| Self::advance(&mut self.read_and_length, ev))
603+
.find_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
590604
}
591605

592606
fn size_hint(&self) -> (usize, Option<usize>) {
@@ -600,13 +614,13 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
600614
F: FnMut(B, Self::Item) -> B,
601615
{
602616
self.messages
603-
.filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
617+
.filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
604618
.fold(init, f)
605619
}
606620

607621
fn count(mut self) -> usize {
608622
self.messages
609-
.filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
623+
.filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
610624
.count()
611625
}
612626

@@ -615,7 +629,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
615629
Self: Sized,
616630
{
617631
self.messages
618-
.filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
632+
.filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
619633
.last()
620634
}
621635

@@ -624,7 +638,7 @@ impl<'buf> Iterator for AncillaryDrain<'buf> {
624638
Self: Sized,
625639
{
626640
self.messages
627-
.filter_map(|ev| Self::advance(&mut self.read_and_length, ev))
641+
.filter_map(|(msg, space)| Self::advance(&mut self.read_and_length, msg, space))
628642
.collect()
629643
}
630644
}
@@ -951,13 +965,21 @@ mod messages {
951965
}
952966

953967
impl<'a> Iterator for Messages<'a> {
954-
type Item = &'a mut c::cmsghdr;
968+
/// A message header, along with the number of bytes of buffer space
969+
/// at and after it, which is an upper bound on the size of the
970+
/// message.
971+
type Item = (&'a mut c::cmsghdr, usize);
955972

956973
#[inline]
957974
fn next(&mut self) -> Option<Self::Item> {
958975
// Get the current header.
959976
let header = self.header?;
960977

978+
// Compute the number of bytes of buffer space at and after this
979+
// header.
980+
let end = (self.msghdr.msg_control as usize) + (self.msghdr.msg_controllen as usize);
981+
let space = end.saturating_sub(header.as_ptr() as usize);
982+
961983
// Get the next header.
962984
self.header = NonNull::new(unsafe { c::CMSG_NXTHDR(&self.msghdr, header.as_ptr()) });
963985

@@ -967,7 +989,7 @@ mod messages {
967989
}
968990

969991
// SAFETY: The lifetime of `header` is tied to this.
970-
Some(unsafe { &mut *header.as_ptr() })
992+
Some((unsafe { &mut *header.as_ptr() }, space))
971993
}
972994

973995
fn size_hint(&self) -> (usize, Option<usize>) {

tests/net/cmsg.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,101 @@ fn test_buffer_sizes() {
3333
assert!(cmsg_space!(ScmRights(1)) * 2 >= cmsg_space!(ScmRights(1), ScmRights(1)));
3434
assert!(cmsg_space!(ScmRights(1), ScmRights(0)) >= cmsg_space!(ScmRights(1)));
3535
}
36+
37+
/// Test that receiving more `SCM_RIGHTS` file descriptors than fit in the
38+
/// ancillary buffer doesn't panic and doesn't produce descriptors from
39+
/// outside the buffer.
40+
///
41+
/// Linux truncates the control data and adjusts `cmsg_len` to match, but
42+
/// macOS truncates the data and leaves `cmsg_len` holding the untruncated
43+
/// length, so the parsing code has to be prepared for a `cmsg_len` that runs
44+
/// past the end of the buffer.
45+
///
46+
/// Platforms report the truncation with `CTRUNC`, but not all of the
47+
/// environments rustix's CI runs in do, so this doesn't check for it.
48+
#[test]
49+
fn test_truncated_scm_rights() {
50+
use rustix::fd::{AsFd, OwnedFd};
51+
use rustix::io::{IoSlice, IoSliceMut};
52+
use rustix::net::{
53+
recvmsg, sendmsg, socket, socketpair, AddressFamily, RecvAncillaryBuffer,
54+
RecvAncillaryMessage, RecvFlags, SendAncillaryBuffer, SendAncillaryMessage, SendFlags,
55+
SocketFlags, SocketType,
56+
};
57+
use std::mem::MaybeUninit;
58+
59+
/// The number of file descriptors to send.
60+
const NUM_FDS: usize = 16;
61+
/// The number of file descriptors the receive buffer has room for.
62+
const NUM_SLOTS: usize = 5;
63+
64+
crate::init();
65+
66+
let (send_sock, recv_sock) = socketpair(
67+
AddressFamily::UNIX,
68+
SocketType::STREAM,
69+
SocketFlags::empty(),
70+
None,
71+
)
72+
.unwrap();
73+
74+
// Make some file descriptors to send.
75+
let fds: Vec<OwnedFd> = (0..NUM_FDS)
76+
.map(|_| socket(AddressFamily::UNIX, SocketType::STREAM, None).unwrap())
77+
.collect();
78+
let borrowed: Vec<_> = fds.iter().map(AsFd::as_fd).collect();
79+
80+
let mut space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_FDS))];
81+
let mut cmsg_buffer = SendAncillaryBuffer::new(space.as_mut_slice());
82+
assert!(cmsg_buffer.push(SendAncillaryMessage::ScmRights(&borrowed)));
83+
84+
sendmsg(
85+
&send_sock,
86+
&[IoSlice::new(b"hello")],
87+
&mut cmsg_buffer,
88+
SendFlags::empty(),
89+
)
90+
.unwrap();
91+
92+
// Receive into a buffer with room for only `NUM_SLOTS` file descriptors.
93+
let mut cmsg_space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(NUM_SLOTS))];
94+
let mut cmsg_buffer = RecvAncillaryBuffer::new(cmsg_space.as_mut_slice());
95+
96+
let mut buffer = [0_u8; 5];
97+
let result = recvmsg(
98+
&recv_sock,
99+
&mut [IoSliceMut::new(&mut buffer)],
100+
&mut cmsg_buffer,
101+
RecvFlags::empty(),
102+
)
103+
.unwrap();
104+
105+
assert_eq!(result.bytes, 5);
106+
assert_eq!(&buffer, b"hello");
107+
108+
// Draining the buffer shouldn't panic.
109+
let mut received = Vec::new();
110+
for msg in cmsg_buffer.drain() {
111+
match msg {
112+
RecvAncillaryMessage::ScmRights(rights) => received.extend(rights),
113+
_ => panic!("unexpected ancillary message"),
114+
}
115+
}
116+
117+
// Platforms deliver differing amounts of a truncated control message —
118+
// Linux fills the buffer, FreeBSD delivers none of it — so don't assume
119+
// how many descriptors come back. Do require that the message was
120+
// truncated, and that every descriptor that did come back is one of the
121+
// sockets that was sent, rather than something read from past the end of
122+
// the buffer.
123+
assert!(received.len() < NUM_FDS);
124+
for fd in &received {
125+
assert_eq!(
126+
rustix::net::sockopt::socket_type(fd).unwrap(),
127+
SocketType::STREAM
128+
);
129+
}
130+
131+
// Dropping the buffer drains it again; that shouldn't panic either.
132+
drop(cmsg_buffer);
133+
}

0 commit comments

Comments
 (0)