From 31fda8f7cd111d2e1b1b87d694442aa62eb82a82 Mon Sep 17 00:00:00 2001 From: Thibaut Vandervelden Date: Fri, 3 Jan 2025 11:43:31 +0100 Subject: [PATCH] fix lladdr parse panic The `RawHardwareAddress::parse()` method panics if the length of the address is invalid. More specific, for an Ethernet address, the length should exactly be 6 bytes, and for an IEEE 802.15.4 address, the length should exactly be 8 bytes. Previously, we only checked if the size of the input was at least the size of the link layer address. Since `Ethernet::from_bytes()` does a copy_from_slice, the length of the input should be exactly 6 bytes. A panic can be triggered when the length is for example 7 bytes, while the `medium-ethernet` and `medium-ieee802154` features are enabled. This commit fixes the panic by checking if the length of the input is exactly the size of the link layer address. This panic was discovered by people from Radically Open Security. Signed-off-by: Thibaut Vandervelden --- src/wire/mod.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/wire/mod.rs b/src/wire/mod.rs index a6db11c3d..478f0cffc 100644 --- a/src/wire/mod.rs +++ b/src/wire/mod.rs @@ -478,6 +478,10 @@ pub struct RawHardwareAddress { #[cfg(any(feature = "medium-ethernet", feature = "medium-ieee802154"))] impl RawHardwareAddress { + /// Create a new `RawHardwareAddress` from a byte slice. + /// + /// # Panics + /// Panics if `addr.len() > MAX_HARDWARE_ADDRESS_LEN`. pub fn from_bytes(addr: &[u8]) -> Self { let mut data = [0u8; MAX_HARDWARE_ADDRESS_LEN]; data[..addr.len()].copy_from_slice(addr); @@ -504,7 +508,7 @@ impl RawHardwareAddress { match medium { #[cfg(feature = "medium-ethernet")] Medium::Ethernet => { - if self.len() < 6 { + if self.len() != 6 { return Err(Error); } Ok(HardwareAddress::Ethernet(EthernetAddress::from_bytes( @@ -513,7 +517,7 @@ impl RawHardwareAddress { } #[cfg(feature = "medium-ieee802154")] Medium::Ieee802154 => { - if self.len() < 8 { + if self.len() != 8 { return Err(Error); } Ok(HardwareAddress::Ieee802154(Ieee802154Address::from_bytes( @@ -559,3 +563,37 @@ impl From for RawHardwareAddress { Self::from_bytes(addr.as_bytes()) } } + +#[cfg(test)] +mod tests { + use super::*; + use rstest::rstest; + + #[rstest] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[0u8; 6][..]), Ok(HardwareAddress::Ethernet(EthernetAddress([0, 0, 0, 0, 0, 0]))))] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[1u8; 5][..]), Err(Error))] + #[cfg(feature = "medium-ethernet")] + #[case((Medium::Ethernet, &[1u8; 7][..]), Err(Error))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[0u8; 8][..]), Ok(HardwareAddress::Ieee802154(Ieee802154Address::Extended([0, 0, 0, 0, 0, 0, 0, 0]))))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[1u8; 2][..]), Err(Error))] + #[cfg(feature = "medium-ieee802154")] + #[case((Medium::Ieee802154, &[1u8; 1][..]), Err(Error))] + fn parse_hardware_address( + #[case] input: (Medium, &[u8]), + #[case] expected: Result, + ) { + let (medium, input) = input; + + // NOTE: we check the length since `RawHardwareAddress::parse()` panics if the length is + // invalid. MAX_HARDWARE_ADDRESS_LEN is based on the medium, and depending on the feature + // flags, it can be different. + if input.len() < MAX_HARDWARE_ADDRESS_LEN { + let raw = RawHardwareAddress::from_bytes(input); + assert_eq!(raw.parse(medium), expected); + } + } +}