From 81e3d3862fa862772a49ccf257fd2709067db823 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:53:44 +0000 Subject: [PATCH 1/2] Reject PDUs whose length is shorter than their fixed fields A root layer, data packet framing layer or DMP layer whose length field was smaller than the fixed fields of that layer caused an out-of-bounds slice and a panic while parsing. Any host on the network could crash a receiver with a single malformed datagram. Each of these layers now returns PduInvalidLength when its length can't hold its own header, as E1.31 requires malformed packets to be discarded. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs --- src/packet.rs | 17 ++++++ tests/malformed_length_tests.rs | 94 +++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 tests/malformed_length_tests.rs diff --git a/src/packet.rs b/src/packet.rs index 74789d56..14ac22c7 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -603,6 +603,13 @@ macro_rules! impl_e131_root_layer { )); } + // The PDU must at least hold the flags/length, vector and CID fields. + if length < E131_CID_END_INDEX { + return Err(SacnError::SacnParsePackError( + ParsePacketError::PduInvalidLength(length), + )); + } + if vector != VECTOR_ROOT_E131_DATA && vector != VECTOR_ROOT_E131_EXTENDED { return Err(SacnError::SacnParsePackError( ParsePacketError::PduInvalidVector(vector), @@ -764,6 +771,11 @@ macro_rules! impl_data_packet_framing_layer { return Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidVector(vector))); } + // The PDU must at least hold every fixed framing layer field before the DMP layer. + if length < DATA_INDEX { + return Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidLength(length))); + } + // Source Name let source_name = String::from( parse_source_name_str( @@ -961,6 +973,11 @@ macro_rules! impl_data_packet_dmp_layer { return Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidVector(vector))); } + // The PDU must at least hold every fixed DMP layer field before the property values. + if length < PROPERTY_VALUES_FIELD_INDEX { + return Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidLength(length))); + } + // Address and Data Type if buf[ADDRESS_DATA_FIELD_INDEX] != E131_DMP_LAYER_ADDRESS_DATA_FIELD { return Err(SacnError::SacnParsePackError(ParsePacketError::ParseInvalidData("invalid Address and Data Type".to_string()))); diff --git a/tests/malformed_length_tests.rs b/tests/malformed_length_tests.rs new file mode 100644 index 00000000..d3eb9d19 --- /dev/null +++ b/tests/malformed_length_tests.rs @@ -0,0 +1,94 @@ +// Copyright 2020 sacn Developers +// +// Licensed under the Apache License, Version 2.0, or the MIT license , at your option. This file may not be +// copied, modified, or distributed except according to those terms. + +//! Packets whose PDU length fields are shorter than the fixed fields of that layer must be +//! rejected with an error rather than causing a panic. + +use sacn::error::errors::SacnError; +use sacn::packet::AcnRootLayerProtocol; +use sacn::sacn_parse_pack_error::ParsePacketError; + +/// Preamble size, post-amble size and ACN packet identifier. +fn acn_header() -> Vec { + let mut v = vec![0x00, 0x10, 0x00, 0x00]; + v.extend_from_slice(b"ASC-E1.17\0\0\0"); + v +} + +fn assert_invalid_length(buf: &[u8]) { + match AcnRootLayerProtocol::parse(buf) { + Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidLength(_))) => {} + other => panic!("expected PduInvalidLength, got {other:?}"), + } +} + +#[test] +fn test_root_layer_length_shorter_than_cid() { + let mut buf = acn_header(); + // Root layer length of 6 covers only flags/length and vector, not the CID. + buf.extend_from_slice(&[0x70, 0x06, 0x00, 0x00, 0x00, 0x04]); + buf.extend_from_slice(&[0u8; 40]); + assert_invalid_length(&buf); +} + +#[test] +fn test_data_framing_layer_length_shorter_than_header() { + let mut buf = acn_header(); + // Root layer: length 28 = 22 header bytes + 6 framing bytes. + buf.extend_from_slice(&[0x70, 0x1c, 0x00, 0x00, 0x00, 0x04]); + buf.extend_from_slice(&[0u8; 16]); // CID + // Data framing layer of length 6: flags/length and vector only. + buf.extend_from_slice(&[0x70, 0x06, 0x00, 0x00, 0x00, 0x02]); + buf.extend_from_slice(&[0u8; 100]); + assert_invalid_length(&buf); +} + +#[test] +fn test_dmp_layer_length_shorter_than_header() { + let framing_len: u8 = 77 + 3; + let mut buf = acn_header(); + buf.extend_from_slice(&[0x70, 22 + framing_len, 0x00, 0x00, 0x00, 0x04]); + buf.extend_from_slice(&[0u8; 16]); // CID + // Data framing layer header. + buf.extend_from_slice(&[0x70, framing_len, 0x00, 0x00, 0x00, 0x02]); + buf.extend_from_slice(&[0u8; 64]); // Source name + buf.extend_from_slice(&[100, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01]); // Priority, sync addr, seq, options, universe 1 + // DMP layer of length 3: flags/length and vector only. + buf.extend_from_slice(&[0x70, 0x03, 0x02]); + buf.extend_from_slice(&[0u8; 20]); + assert_invalid_length(&buf); +} + +/// Every truncation of the PDU length fields of a valid data packet must return an error, never panic. +#[test] +fn test_all_short_lengths_rejected_without_panic() { + let mut valid = acn_header(); + let dmp_len: u16 = 10 + 513; + let framing_len: u16 = 77 + dmp_len; + let root_len: u16 = 22 + framing_len; + valid.extend_from_slice(&(0x7000 | root_len).to_be_bytes()); + valid.extend_from_slice(&[0x00, 0x00, 0x00, 0x04]); + valid.extend_from_slice(&[0u8; 16]); + valid.extend_from_slice(&(0x7000 | framing_len).to_be_bytes()); + valid.extend_from_slice(&[0x00, 0x00, 0x00, 0x02]); + valid.extend_from_slice(&[0u8; 64]); + valid.extend_from_slice(&[100, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01]); + valid.extend_from_slice(&(0x7000 | dmp_len).to_be_bytes()); + valid.extend_from_slice(&[0x02, 0xa1, 0x00, 0x00, 0x00, 0x01]); + valid.extend_from_slice(&513u16.to_be_bytes()); + valid.extend_from_slice(&[0u8; 513]); + assert!(AcnRootLayerProtocol::parse(&valid).is_ok()); + + // Offsets of the root, framing and DMP flags/length fields. + for offset in [16usize, 38, 115] { + for len in 0u16..0x0fff { + let mut buf = valid.clone(); + buf[offset..offset + 2].copy_from_slice(&(0x7000 | len).to_be_bytes()); + let _ = AcnRootLayerProtocol::parse(&buf); + } + } +} From 46add3232cde01c8dbfa94ead977dbbfa5d8e2be Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:54:09 +0000 Subject: [PATCH 2/2] Keep ParseInsufficientData for a short DMP layer length The existing DMP layer tests expect ParseInsufficientData when the PDU length is too low, so keep that error type for the new header check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs --- src/packet.rs | 4 +++- tests/malformed_length_tests.rs | 9 ++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/packet.rs b/src/packet.rs index 14ac22c7..377e4e33 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -975,7 +975,9 @@ macro_rules! impl_data_packet_dmp_layer { // The PDU must at least hold every fixed DMP layer field before the property values. if length < PROPERTY_VALUES_FIELD_INDEX { - return Err(SacnError::SacnParsePackError(ParsePacketError::PduInvalidLength(length))); + return Err(SacnError::SacnParsePackError(ParsePacketError::ParseInsufficientData( + format!("Data packet dmp layer pdu length {} is shorter than its {} byte header", length, PROPERTY_VALUES_FIELD_INDEX) + ))); } // Address and Data Type diff --git a/tests/malformed_length_tests.rs b/tests/malformed_length_tests.rs index d3eb9d19..fe1701e9 100644 --- a/tests/malformed_length_tests.rs +++ b/tests/malformed_length_tests.rs @@ -26,6 +26,13 @@ fn assert_invalid_length(buf: &[u8]) { } } +fn assert_insufficient_data(buf: &[u8]) { + match AcnRootLayerProtocol::parse(buf) { + Err(SacnError::SacnParsePackError(ParsePacketError::ParseInsufficientData(_))) => {} + other => panic!("expected ParseInsufficientData, got {other:?}"), + } +} + #[test] fn test_root_layer_length_shorter_than_cid() { let mut buf = acn_header(); @@ -60,7 +67,7 @@ fn test_dmp_layer_length_shorter_than_header() { // DMP layer of length 3: flags/length and vector only. buf.extend_from_slice(&[0x70, 0x03, 0x02]); buf.extend_from_slice(&[0u8; 20]); - assert_invalid_length(&buf); + assert_insufficient_data(&buf); } /// Every truncation of the PDU length fields of a valid data packet must return an error, never panic.