From 9470bac18968890f904090d746e79b987420e6e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:56:32 +0000 Subject: [PATCH] Parse only the bytes of the received datagram recv read each datagram into a zero-filled 1144 byte buffer and then parsed the whole buffer, ignoring how many bytes actually arrived. A datagram cut short in transit was padded out with zeros and accepted instead of being discarded as malformed. Parse only the first n bytes, through a shared parse_datagram helper used by both the Windows and Unix receivers. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs --- src/receive.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/receive.rs b/src/receive.rs index 15f4141..e6b3758 100644 --- a/src/receive.rs +++ b/src/receive.rs @@ -1017,6 +1017,22 @@ impl Drop for SacnReceiver { /// srcs: The Vec of `DiscoveredSacnSources` to search. /// /// cid: The CID (uuid) of the source to find. +/// Parses the `n` bytes of a received datagram held at the start of `buf`. +/// +/// Only the bytes actually received are parsed, so a truncated datagram is rejected by the length checks +/// in the parser rather than being padded out with whatever is left in the rest of the buffer. +/// +/// # Errors +/// `TooManyBytesRead`: Returned if `n` is larger than the buffer. +/// +/// Otherwise returns any error from `AcnRootLayerProtocol::parse`. +fn parse_datagram(buf: &[u8], n: usize) -> Result> { + if n > buf.len() { + return Err(SacnError::TooManyBytesRead(n, buf.len())); + } + AcnRootLayerProtocol::parse(&buf[..n]) +} + fn find_discovered_src(srcs: &[DiscoveredSacnSource], cid: &Uuid) -> Option { (0..srcs.len()).find(|&i| srcs[i].cid == *cid) } @@ -1138,10 +1154,7 @@ impl SacnNetworkReceiver { ) -> Result> { // use read() for the windows impl, since windows does not like using read_exact() let n = self.socket.read(buf)?; - if n > RCV_BUF_DEFAULT_SIZE { - return Err(SacnError::TooManyBytesRead(n, buf.len())); - } - AcnRootLayerProtocol::parse(buf) + parse_datagram(buf, n) } /// Set the timeout for the recv operation. @@ -1271,10 +1284,7 @@ impl SacnNetworkReceiver { ) -> Result> { // use read() since read_exact() was not passing the tests. let n = self.socket.read(buf)?; - if n > RCV_BUF_DEFAULT_SIZE { - return Err(SacnError::TooManyBytesRead(n, buf.len())); - } - AcnRootLayerProtocol::parse(buf) + parse_datagram(buf, n) } /// Set the timeout for the recv operation. @@ -2120,6 +2130,49 @@ pub fn htp_dmx_merge(i: &DMXData, n: &DMXData) -> Result { mod test { use super::*; + /// A valid data packet whose DMX data was cut short in transit must be rejected rather than parsed with + /// zeros filling in for the missing bytes. + #[test] + fn test_parse_datagram_rejects_truncated_packet() { + let packet = AcnRootLayerProtocol { + pdu: E131RootLayer { + cid: Uuid::new_v4(), + data: E131RootLayerData::DataPacket(DataPacketFramingLayer { + source_name: "Source".into(), + priority: E131_DEFAULT_PRIORITY, + synchronization_address: 0, + sequence_number: 0, + preview_data: false, + stream_terminated: false, + force_synchronization: false, + universe: 1, + data: DataPacketDmpLayer { + property_values: vec![0u8; 513].into(), + }, + }), + }, + }; + let packed = packet.pack_alloc().unwrap(); + + // The receiver always reads into a zero-filled buffer of the maximum packet size. + let mut buf = [0u8; RCV_BUF_DEFAULT_SIZE]; + buf[..packed.len()].copy_from_slice(&packed); + + assert_eq!(parse_datagram(&buf, packed.len()).unwrap(), packet); + + // The last 100 DMX slots never arrived. + assert!(parse_datagram(&buf, packed.len() - 100).is_err()); + } + + #[test] + fn test_parse_datagram_rejects_length_larger_than_buffer() { + let buf = [0u8; RCV_BUF_DEFAULT_SIZE]; + match parse_datagram(&buf, RCV_BUF_DEFAULT_SIZE + 1) { + Err(SacnError::TooManyBytesRead(_, _)) => {} + other => panic!("Expected TooManyBytesRead, got {other:?}"), + } + } + use std::borrow::Cow; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::time::Instant;