Skip to content

Commit 1386417

Browse files
thvdveldthvdveld
committed
fix(socket): truncation when calling recv_slice
When calling `recv_slice`/`peek_slice` in the udp, icmp and raw sockets, data is lost when the provided buffer is smaller than the payload to be copied (not the case for `peek_slice` because it is not dequeued). With this commit, an `RecvError::Truncated` error is returned. In case of UDP, the endpoint is also returned in the error. In case of ICMP, the IP address is also returned in the error. I implemented it the way Whitequark proposes it. Data is still lost, but at least the caller knows that the data was truncated to the size of the provided buffer. As Whitequark says, it is preferred to call `recv` instead of `recv_slice` as there would be no truncation.
1 parent a5da783 commit 1386417

File tree

4 files changed

+91
-26
lines changed

4 files changed

+91
-26
lines changed

‎src/socket/icmp.rs

+50-4
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,14 @@ impl std::error::Error for SendError {}
6161
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
6262
pub enum RecvError {
6363
Exhausted,
64+
Truncated,
6465
}
6566

6667
impl core::fmt::Display for RecvError {
6768
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
6869
match self {
6970
RecvError::Exhausted => write!(f, "exhausted"),
71+
RecvError::Truncated => write!(f, "truncated"),
7072
}
7173
}
7274
}
@@ -130,8 +132,8 @@ impl<'a> Socket<'a> {
130132
/// Create an ICMP socket with the given buffers.
131133
pub fn new(rx_buffer: PacketBuffer<'a>, tx_buffer: PacketBuffer<'a>) -> Socket<'a> {
132134
Socket {
133-
rx_buffer: rx_buffer,
134-
tx_buffer: tx_buffer,
135+
rx_buffer,
136+
tx_buffer,
135137
endpoint: Default::default(),
136138
hop_limit: None,
137139
#[cfg(feature = "async")]
@@ -394,9 +396,17 @@ impl<'a> Socket<'a> {
394396
/// Dequeue a packet received from a remote endpoint, copy the payload into the given slice,
395397
/// and return the amount of octets copied as well as the `IpAddress`
396398
///
399+
/// **Note**: when the size of the provided buffer is smaller than the size of the payload,
400+
/// the packet is dropped and a `RecvError::Truncated` error is returned.
401+
///
397402
/// See also [recv](#method.recv).
398403
pub fn recv_slice(&mut self, data: &mut [u8]) -> Result<(usize, IpAddress), RecvError> {
399404
let (buffer, endpoint) = self.recv()?;
405+
406+
if data.len() < buffer.len() {
407+
return Err(RecvError::Truncated);
408+
}
409+
400410
let length = cmp::min(data.len(), buffer.len());
401411
data[..length].copy_from_slice(&buffer[..length]);
402412
Ok((length, endpoint))
@@ -555,7 +565,7 @@ impl<'a> Socket<'a> {
555565
dst_addr,
556566
next_header: IpProtocol::Icmp,
557567
payload_len: repr.buffer_len(),
558-
hop_limit: hop_limit,
568+
hop_limit,
559569
});
560570
emit(cx, (ip_repr, IcmpRepr::Ipv4(repr)))
561571
}
@@ -592,7 +602,7 @@ impl<'a> Socket<'a> {
592602
dst_addr,
593603
next_header: IpProtocol::Icmpv6,
594604
payload_len: repr.buffer_len(),
595-
hop_limit: hop_limit,
605+
hop_limit,
596606
});
597607
emit(cx, (ip_repr, IcmpRepr::Ipv6(repr)))
598608
}
@@ -1096,6 +1106,42 @@ mod test_ipv6 {
10961106
assert!(!socket.can_recv());
10971107
}
10981108

1109+
#[rstest]
1110+
#[case::ethernet(Medium::Ethernet)]
1111+
#[cfg(feature = "medium-ethernet")]
1112+
fn test_truncated_recv_slice(#[case] medium: Medium) {
1113+
let (mut iface, _, _) = setup(medium);
1114+
let cx = iface.context();
1115+
1116+
let mut socket = socket(buffer(1), buffer(1));
1117+
assert_eq!(socket.bind(Endpoint::Ident(0x1234)), Ok(()));
1118+
1119+
let checksum = ChecksumCapabilities::default();
1120+
1121+
let mut bytes = [0xff; 24];
1122+
let mut packet = Icmpv6Packet::new_unchecked(&mut bytes[..]);
1123+
ECHOV6_REPR.emit(
1124+
&LOCAL_IPV6.into(),
1125+
&REMOTE_IPV6.into(),
1126+
&mut packet,
1127+
&checksum,
1128+
);
1129+
1130+
assert!(socket.accepts(cx, &REMOTE_IPV6_REPR, &ECHOV6_REPR.into()));
1131+
socket.process(cx, &REMOTE_IPV6_REPR, &ECHOV6_REPR.into());
1132+
assert!(socket.can_recv());
1133+
1134+
assert!(socket.accepts(cx, &REMOTE_IPV6_REPR, &ECHOV6_REPR.into()));
1135+
socket.process(cx, &REMOTE_IPV6_REPR, &ECHOV6_REPR.into());
1136+
1137+
let mut buffer = [0u8; 1];
1138+
assert_eq!(
1139+
socket.recv_slice(&mut buffer[..]),
1140+
Err(RecvError::Truncated)
1141+
);
1142+
assert!(!socket.can_recv());
1143+
}
1144+
10991145
#[rstest]
11001146
#[case::ethernet(Medium::Ethernet)]
11011147
#[cfg(feature = "medium-ethernet")]

‎src/socket/raw.rs

+19-6
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,14 @@ impl std::error::Error for SendError {}
5757
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
5858
pub enum RecvError {
5959
Exhausted,
60+
Truncated,
6061
}
6162

6263
impl core::fmt::Display for RecvError {
6364
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
6465
match self {
6566
RecvError::Exhausted => write!(f, "exhausted"),
67+
RecvError::Truncated => write!(f, "truncated"),
6668
}
6769
}
6870
}
@@ -273,9 +275,16 @@ impl<'a> Socket<'a> {
273275

274276
/// Dequeue a packet, and copy the payload into the given slice.
275277
///
278+
/// **Note**: when the size of the provided buffer is smaller than the size of the payload,
279+
/// the packet is dropped and a `RecvError::Truncated` error is returned.
280+
///
276281
/// See also [recv](#method.recv).
277282
pub fn recv_slice(&mut self, data: &mut [u8]) -> Result<usize, RecvError> {
278283
let buffer = self.recv()?;
284+
if data.len() < buffer.len() {
285+
return Err(RecvError::Truncated);
286+
}
287+
279288
let length = min(data.len(), buffer.len());
280289
data[..length].copy_from_slice(&buffer[..length]);
281290
Ok(length)
@@ -303,9 +312,16 @@ impl<'a> Socket<'a> {
303312
/// and return the amount of octets copied without removing the packet from the receive buffer.
304313
/// This function otherwise behaves identically to [recv_slice](#method.recv_slice).
305314
///
315+
/// **Note**: when the size of the provided buffer is smaller than the size of the payload,
316+
/// no data is copied into the provided buffer and a `RecvError::Truncated` error is returned.
317+
///
306318
/// See also [peek](#method.peek).
307319
pub fn peek_slice(&mut self, data: &mut [u8]) -> Result<usize, RecvError> {
308320
let buffer = self.peek()?;
321+
if data.len() < buffer.len() {
322+
return Err(RecvError::Truncated);
323+
}
324+
309325
let length = min(data.len(), buffer.len());
310326
data[..length].copy_from_slice(&buffer[..length]);
311327
Ok(length)
@@ -602,8 +618,7 @@ mod test {
602618
socket.process(&mut cx, &$hdr, &$payload);
603619

604620
let mut slice = [0; 4];
605-
assert_eq!(socket.recv_slice(&mut slice[..]), Ok(4));
606-
assert_eq!(&slice, &$packet[..slice.len()]);
621+
assert_eq!(socket.recv_slice(&mut slice[..]), Err(RecvError::Truncated));
607622
}
608623

609624
#[rstest]
@@ -641,10 +656,8 @@ mod test {
641656
socket.process(&mut cx, &$hdr, &$payload);
642657

643658
let mut slice = [0; 4];
644-
assert_eq!(socket.peek_slice(&mut slice[..]), Ok(4));
645-
assert_eq!(&slice, &$packet[..slice.len()]);
646-
assert_eq!(socket.recv_slice(&mut slice[..]), Ok(4));
647-
assert_eq!(&slice, &$packet[..slice.len()]);
659+
assert_eq!(socket.peek_slice(&mut slice[..]), Err(RecvError::Truncated));
660+
assert_eq!(socket.recv_slice(&mut slice[..]), Err(RecvError::Truncated));
648661
assert_eq!(socket.peek_slice(&mut slice[..]), Err(RecvError::Exhausted));
649662
}
650663
}

‎src/socket/tcp.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1331,7 +1331,7 @@ impl<'a> Socket<'a> {
13311331
// Rate-limit to 1 per second max.
13321332
self.challenge_ack_timer = cx.now() + Duration::from_secs(1);
13331333

1334-
return Some(self.ack_reply(ip_repr, repr));
1334+
Some(self.ack_reply(ip_repr, repr))
13351335
}
13361336

13371337
pub(crate) fn accepts(&self, _cx: &mut Context, ip_repr: &IpRepr, repr: &TcpRepr) -> bool {

‎src/socket/udp.rs

+21-15
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,14 @@ impl std::error::Error for SendError {}
8888
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
8989
pub enum RecvError {
9090
Exhausted,
91+
Truncated,
9192
}
9293

9394
impl core::fmt::Display for RecvError {
9495
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
9596
match self {
9697
RecvError::Exhausted => write!(f, "exhausted"),
98+
RecvError::Truncated => write!(f, "truncated"),
9799
}
98100
}
99101
}
@@ -393,9 +395,17 @@ impl<'a> Socket<'a> {
393395
/// Dequeue a packet received from a remote endpoint, copy the payload into the given slice,
394396
/// and return the amount of octets copied as well as the endpoint.
395397
///
398+
/// **Note**: when the size of the provided buffer is smaller than the size of the payload,
399+
/// the packet is dropped and a `RecvError::Truncated` error is returned.
400+
///
396401
/// See also [recv](#method.recv).
397402
pub fn recv_slice(&mut self, data: &mut [u8]) -> Result<(usize, UdpMetadata), RecvError> {
398403
let (buffer, endpoint) = self.recv().map_err(|_| RecvError::Exhausted)?;
404+
405+
if data.len() < buffer.len() {
406+
return Err(RecvError::Truncated);
407+
}
408+
399409
let length = min(data.len(), buffer.len());
400410
data[..length].copy_from_slice(&buffer[..length]);
401411
Ok((length, endpoint))
@@ -426,9 +436,17 @@ impl<'a> Socket<'a> {
426436
/// packet from the receive buffer.
427437
/// This function otherwise behaves identically to [recv_slice](#method.recv_slice).
428438
///
439+
/// **Note**: when the size of the provided buffer is smaller than the size of the payload,
440+
/// no data is copied into the provided buffer and a `RecvError::Truncated` error is returned.
441+
///
429442
/// See also [peek](#method.peek).
430443
pub fn peek_slice(&mut self, data: &mut [u8]) -> Result<(usize, &UdpMetadata), RecvError> {
431444
let (buffer, endpoint) = self.peek()?;
445+
446+
if data.len() < buffer.len() {
447+
return Err(RecvError::Truncated);
448+
}
449+
432450
let length = min(data.len(), buffer.len());
433451
data[..length].copy_from_slice(&buffer[..length]);
434452
Ok((length, endpoint))
@@ -851,11 +869,7 @@ mod test {
851869
);
852870

853871
let mut slice = [0; 4];
854-
assert_eq!(
855-
socket.recv_slice(&mut slice[..]),
856-
Ok((4, REMOTE_END.into()))
857-
);
858-
assert_eq!(&slice, b"abcd");
872+
assert_eq!(socket.recv_slice(&mut slice[..]), Err(RecvError::Truncated));
859873
}
860874

861875
#[rstest]
@@ -882,16 +896,8 @@ mod test {
882896
);
883897

884898
let mut slice = [0; 4];
885-
assert_eq!(
886-
socket.peek_slice(&mut slice[..]),
887-
Ok((4, &REMOTE_END.into()))
888-
);
889-
assert_eq!(&slice, b"abcd");
890-
assert_eq!(
891-
socket.recv_slice(&mut slice[..]),
892-
Ok((4, REMOTE_END.into()))
893-
);
894-
assert_eq!(&slice, b"abcd");
899+
assert_eq!(socket.peek_slice(&mut slice[..]), Err(RecvError::Truncated));
900+
assert_eq!(socket.recv_slice(&mut slice[..]), Err(RecvError::Truncated));
895901
assert_eq!(socket.peek_slice(&mut slice[..]), Err(RecvError::Exhausted));
896902
}
897903

0 commit comments

Comments
 (0)