From 84ba66f636a22afef38739aa7c85066dab3f098e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:55:20 +0000 Subject: [PATCH 1/2] Limit source names to 63 bytes so they stay null-terminated E1.31-2016 Section 6.2.2 requires the 64 byte Source Name field to be null-terminated, so a name can be at most 63 bytes. SacnSource accepted 64 byte names and sent them with no terminator, which this crate's own receiver then rejected with SourceNameNotNullTerminated. Packing a name of 65 bytes or more panicked. Add E131_SOURCE_NAME_MAX_LENGTH (63), check it when creating or renaming a source, and return PackInvalidData from the data and discovery framing layer pack functions when a name is too long. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs --- src/packet.rs | 19 ++++++++- src/source.rs | 10 ++--- tests/src_tests.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/packet.rs b/src/packet.rs index 74789d56..c79d09ae 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -248,6 +248,11 @@ const E131_CID_END_INDEX: usize = /// The length of the Source Name field in bytes in an ANSI E1.31-2018 packet as per ANSI E1.31-2018 Section 4, Table 4-1, 4-2, 4-3. pub const E131_SOURCE_NAME_FIELD_LENGTH: usize = 64; +/// The maximum length of a source name in bytes (UTF-8), as per ANSI E1.31-2016 Section 6.2.2. +/// +/// The source name field is 64 bytes and must be null-terminated, which leaves 63 bytes for the name itself. +pub const E131_SOURCE_NAME_MAX_LENGTH: usize = E131_SOURCE_NAME_FIELD_LENGTH - 1; + /// The length of the Synchronisation Address field in bytes in an ANSI E1.31-2018 packet as per ANSI E1.31-2018 Section 4, Table 4-1, 4-2, 4-3. pub const E131_SYNC_ADDR_FIELD_LENGTH: usize = 2; @@ -832,7 +837,12 @@ macro_rules! impl_data_packet_framing_layer { // Vector NetworkEndian::write_u32(&mut buf[E131_PDU_LENGTH_FLAGS_LENGTH .. SOURCE_NAME_INDEX], VECTOR_E131_DATA_PACKET); - // Source Name, padded with 0's up to the required 64 byte length. + // Source Name, padded with 0's up to the required 64 byte length, always leaving room for the null terminator. + if self.source_name.len() > E131_SOURCE_NAME_MAX_LENGTH { + return Err(SacnError::SacnParsePackError(ParsePacketError::PackInvalidData( + format!("Source name must be at most {} bytes", E131_SOURCE_NAME_MAX_LENGTH) + ))); + } zeros(&mut buf[SOURCE_NAME_INDEX .. PRIORITY_INDEX], E131_SOURCE_NAME_FIELD_LENGTH); buf[SOURCE_NAME_INDEX .. SOURCE_NAME_INDEX + self.source_name.len()].copy_from_slice(self.source_name.as_bytes()); @@ -1263,7 +1273,12 @@ macro_rules! impl_universe_discovery_packet_framing_layer { // Vector NetworkEndian::write_u32(&mut buf[E131_DISCOVERY_FRAMING_LAYER_VECTOR_FIELD_INDEX .. E131_DISCOVERY_FRAMING_LAYER_SOURCE_NAME_FIELD_INDEX], VECTOR_E131_EXTENDED_DISCOVERY); - // Source Name + // Source Name, always leaving room for the null terminator. + if self.source_name.len() > E131_SOURCE_NAME_MAX_LENGTH { + return Err(SacnError::SacnParsePackError(ParsePacketError::PackInvalidData( + format!("Source name must be at most {} bytes", E131_SOURCE_NAME_MAX_LENGTH) + ))); + } zeros(&mut buf[E131_DISCOVERY_FRAMING_LAYER_SOURCE_NAME_FIELD_INDEX .. E131_DISCOVERY_FRAMING_LAYER_RESERVE_FIELD_INDEX], E131_SOURCE_NAME_FIELD_LENGTH); buf[E131_DISCOVERY_FRAMING_LAYER_SOURCE_NAME_FIELD_INDEX .. E131_DISCOVERY_FRAMING_LAYER_SOURCE_NAME_FIELD_INDEX + self.source_name.len()].copy_from_slice(self.source_name.as_bytes()); diff --git a/src/source.rs b/src/source.rs index f701751b..ccb346d4 100644 --- a/src/source.rs +++ b/src/source.rs @@ -192,9 +192,9 @@ impl SacnSource { /// /// `UnsupportedIpVersion`: Returned if the `SocketAddr` is not IPv4 or IPv6. /// - /// `MalformedSourceName`: Returned if the given source name is longer than the maximum allowed size of `E131_SOURCE_NAME_FIELD_LENGTH`. + /// `MalformedSourceName`: Returned if the given source name is longer than the maximum allowed size of `E131_SOURCE_NAME_MAX_LENGTH` (63 bytes, leaving room for the null terminator). pub fn with_cid_ip(name: &str, cid: Uuid, ip: SocketAddr) -> Result { - if name.len() > E131_SOURCE_NAME_FIELD_LENGTH { + if name.len() > E131_SOURCE_NAME_MAX_LENGTH { return Err(SacnError::MalformedSourceName( "Source name provided is longer than maximum allowed".to_string(), )); @@ -404,7 +404,7 @@ impl SacnSource { /// `SourceCorrupt`: Returned if the Mutex used to control access to the internal sender is poisoned by a thread encountering /// a panic while accessing causing the source to be left in a potentially inconsistent state. /// - /// `MalformedSourceName`: Returned to indicate that the given source name is longer than the maximum allowed as per `E131_SOURCE_NAME_FIELD_LENGTH`. + /// `MalformedSourceName`: Returned to indicate that the given source name is longer than the maximum allowed as per `E131_SOURCE_NAME_MAX_LENGTH` (63 bytes, leaving room for the null terminator). pub fn set_name(&mut self, name: &str) -> Result<()> { unlock_internal_mut(&mut self.internal)?.set_name(name) } @@ -1143,9 +1143,9 @@ impl SacnSourceInternal { /// name: The new name for the source, it is left to the user to ensure this is unique within the sACN network. /// /// # Errors - /// `MalformedSourceName`: Returned to indicate that the given source name is longer than the maximum allowed as per `E131_SOURCE_NAME_FIELD_LENGTH`. + /// `MalformedSourceName`: Returned to indicate that the given source name is longer than the maximum allowed as per `E131_SOURCE_NAME_MAX_LENGTH` (63 bytes, leaving room for the null terminator). fn set_name(&mut self, name: &str) -> Result<()> { - if name.len() > E131_SOURCE_NAME_FIELD_LENGTH { + if name.len() > E131_SOURCE_NAME_MAX_LENGTH { return Err(SacnError::MalformedSourceName( "Source name provided is longer than maximum allowed".to_string(), )); diff --git a/tests/src_tests.rs b/tests/src_tests.rs index ee9605f4..fe8f2485 100644 --- a/tests/src_tests.rs +++ b/tests/src_tests.rs @@ -702,3 +702,100 @@ const TEST_DATA_MULTIPLE_UNIVERSE: [u8; 712] = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 100, ]; + +/// A 64 byte source name fills the whole field and leaves no room for the null terminator required by +/// ANSI E1.31-2016 Section 6.2.2, so it must be rejected. +#[test] +#[serial] +fn test_new_ipv4_64_byte_source_name_rejected() { + let name = "a".repeat(E131_SOURCE_NAME_FIELD_LENGTH); + match SacnSource::new_v4(&name) { + Err(SacnError::MalformedSourceName(_)) => {} + Err(e) => panic!("Unexpected error returned: {e:?}"), + Ok(_) => panic!("SacnSource created with a 64 byte source name"), + } +} + +#[test] +#[serial] +fn test_new_ipv4_63_byte_source_name_accepted() { + let name = "a".repeat(E131_SOURCE_NAME_MAX_LENGTH); + let src = SacnSource::new_v4(&name).unwrap(); + assert_eq!(src.name().unwrap(), name); +} + +#[test] +#[serial] +fn test_set_name_64_byte_source_name_rejected() { + let mut src = SacnSource::new_v4("Source").unwrap(); + match src.set_name(&"a".repeat(E131_SOURCE_NAME_FIELD_LENGTH)) { + Err(SacnError::MalformedSourceName(_)) => {} + other => panic!("Expected MalformedSourceName, got {other:?}"), + } + assert_eq!(src.name().unwrap(), "Source"); +} + +/// A 63 byte source name packs into a packet that parses back to the same name. +#[test] +fn test_pack_parse_63_byte_source_name() { + let name = "a".repeat(E131_SOURCE_NAME_MAX_LENGTH); + let packet = data_packet_with_source_name(&name); + let buf = packet.pack_alloc().unwrap(); + assert_eq!(AcnRootLayerProtocol::parse(&buf).unwrap(), packet); +} + +/// Packing a source name that can't be null-terminated is an error rather than a non-compliant packet +/// (64 bytes) or a panic (65 or more bytes). +#[test] +fn test_pack_too_long_source_name_rejected() { + for len in [E131_SOURCE_NAME_FIELD_LENGTH, E131_SOURCE_NAME_FIELD_LENGTH + 1] { + let name = "a".repeat(len); + match data_packet_with_source_name(&name).pack_alloc() { + Err(SacnError::SacnParsePackError( + sacn::sacn_parse_pack_error::ParsePacketError::PackInvalidData(_), + )) => {} + other => panic!("Expected PackInvalidData for {len} byte name, got {other:?}"), + } + + let discovery = AcnRootLayerProtocol { + pdu: E131RootLayer { + cid: Uuid::new_v4(), + data: E131RootLayerData::UniverseDiscoveryPacket(UniverseDiscoveryPacketFramingLayer { + source_name: name.as_str().into(), + data: UniverseDiscoveryPacketUniverseDiscoveryLayer { + page: 0, + last_page: 0, + universes: vec![1u16].into(), + }, + }), + }, + }; + match discovery.pack_alloc() { + Err(SacnError::SacnParsePackError( + sacn::sacn_parse_pack_error::ParsePacketError::PackInvalidData(_), + )) => {} + other => panic!("Expected PackInvalidData for {len} byte name, got {other:?}"), + } + } +} + +fn data_packet_with_source_name(name: &str) -> AcnRootLayerProtocol<'_> { + AcnRootLayerProtocol { + pdu: E131RootLayer { + cid: Uuid::new_v4(), + data: E131RootLayerData::DataPacket(DataPacketFramingLayer { + source_name: name.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(), + }, + }), + }, + } +} From 3c9e0dbed70e3acd705060d8799a9b5e59da0053 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 09:09:20 +0000 Subject: [PATCH 2/2] Pass unpadded source names to SacnSource in IP tests These tests build the expected 64 byte source name field, padded with nulls, and passed that same padded string to SacnSource. With the 63 byte limit the padded string is rejected. Pass the name without its padding; the packets sent are byte-for-byte the same. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KzaQVQThQpmqhvLapk99Vs --- tests/ipv4_tests.rs | 10 +++++----- tests/ipv6_tests.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/ipv4_tests.rs b/tests/ipv4_tests.rs index 7cf5c28f..bf17e14c 100644 --- a/tests/ipv4_tests.rs +++ b/tests/ipv4_tests.rs @@ -3311,7 +3311,7 @@ fn test_data_packet_transmit_format() { let packet = generate_data_packet_raw(CID, universe, source_name.clone(), PRIORITY, sequence, OPTIONS, dmx_data.clone()); let ip: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), ACN_SDT_MULTICAST_PORT + 1); - let mut source = SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ip).unwrap(); + let mut source = SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ip).unwrap(); source.set_preview_mode(false).unwrap(); source.set_multicast_loop_v4(true).unwrap(); @@ -3509,7 +3509,7 @@ fn test_discovery_packet_transmit_format() { let ip: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), ACN_SDT_MULTICAST_PORT + 1); // Creates the source. - let mut source = SacnSource::with_cid_ip(&str::from_utf8(&SOURCE_NAME).unwrap(), Uuid::from_bytes(CID), ip).unwrap(); + let mut source = SacnSource::with_cid_ip(str::from_utf8(&SOURCE_NAME).unwrap().trim_end_matches('\0'), Uuid::from_bytes(CID), ip).unwrap(); source.set_multicast_loop_v4(true).unwrap(); @@ -3668,7 +3668,7 @@ fn test_track_data_packet_seq_numbers() { // Create a source. let ip: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), ACN_SDT_MULTICAST_PORT + 1); - let mut source = SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ip).unwrap(); + let mut source = SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ip).unwrap(); source.set_multicast_loop_v4(true).unwrap(); source.register_universes(&UNIVERSES).unwrap(); @@ -3735,7 +3735,7 @@ fn test_track_sync_packet_seq_numbers() { // Create a source. let ip: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), ACN_SDT_MULTICAST_PORT + 1); - let mut source = SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ip).unwrap(); + let mut source = SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ip).unwrap(); source.set_multicast_loop_v4(true).unwrap(); // Register the synchronisation addresses. @@ -3810,7 +3810,7 @@ fn test_sync_packet_multicast_address() { // Create a source. let ip: SocketAddr = SocketAddr::new(IpAddr::V4(TEST_NETWORK_INTERFACE_IPV4[0].parse().unwrap()), ACN_SDT_MULTICAST_PORT + 1); - let mut source = SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ip).unwrap(); + let mut source = SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ip).unwrap(); source.set_multicast_loop_v4(true).unwrap(); // Register the synchronisation addresses. diff --git a/tests/ipv6_tests.rs b/tests/ipv6_tests.rs index 55acd344..e3f389a5 100644 --- a/tests/ipv6_tests.rs +++ b/tests/ipv6_tests.rs @@ -1624,7 +1624,7 @@ mod sacn_ipv6_multicast_test { ACN_SDT_MULTICAST_PORT + 1, ); let mut ipv4_source = - SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ipv4).unwrap(); + SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ipv4).unwrap(); ipv4_source.set_preview_mode(false).unwrap(); ipv4_source.set_multicast_loop_v4(true).unwrap(); ipv4_source.register_universes(&[universe as u16]).unwrap(); @@ -1665,7 +1665,7 @@ mod sacn_ipv6_multicast_test { ACN_SDT_MULTICAST_PORT + 1, ); let mut ipv6_source = - SacnSource::with_cid_ip(&source_name.clone(), Uuid::from_bytes(CID), ipv6).unwrap(); + SacnSource::with_cid_ip(source_name.trim_end_matches('\0'), Uuid::from_bytes(CID), ipv6).unwrap(); ipv6_source.set_preview_mode(false).unwrap(); ipv6_source.register_universes(&[universe]).unwrap();