From ef208547b637ef78087f67b7377994dc65c38e75 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Fri, 7 Feb 2025 15:03:33 +0100 Subject: [PATCH] Skip unsupported radiotap fields in the header When unsupported fields (such as HE) appeared in the header they would not be stored in "kinds". This resulted in panics as the fields would later be parsed as VendorNamespace and an invalid length would be stored in skip_length. This commit adds a new enum for unsupported fields so they can be stored and later skipped during parsing. Additional bounds checking is also added for ValueNamespace parsing. --- src/field/mod.rs | 34 ++++++++++++++++------------------ src/lib.rs | 3 +++ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/field/mod.rs b/src/field/mod.rs index cb3ff98..1bcfa07 100644 --- a/src/field/mod.rs +++ b/src/field/mod.rs @@ -37,11 +37,12 @@ pub enum Kind { VHT, Timestamp, VendorNamespace(Option), + Unsupported(u8), } impl Kind { - pub fn new(value: u8) -> Result { - Ok(match value { + pub fn new(value: u8) -> Kind { + match value { 0 => Kind::TSFT, 1 => Kind::Flags, 2 => Kind::Rate, @@ -65,10 +66,9 @@ impl Kind { 20 => Kind::AMPDUStatus, 21 => Kind::VHT, 22 => Kind::Timestamp, - _ => { - return Err(Error::UnsupportedField); - } - }) + 30 => Kind::VendorNamespace(None), + n => Kind::Unsupported(n), + } } /// Returns the align value for the field. @@ -169,21 +169,18 @@ impl Field for Header { present = cursor.read_u32::()?; if !vendor_namespace { - for bit in 0..29 { + for bit in 0..31 { if present.is_bit_set(bit) { - match Kind::new(present_count * 32 + bit) { - Ok(kind) => { - kinds.push(kind); - } - Err(Error::UnsupportedField) => { - // Does not matter, we will just parse the ones we can - } - Err(e) => return Err(e), - } + kinds.push(Kind::new(present_count * 32 + bit)); } } } + // Radiotap and VendorNamespace are exclusive + if present.is_bit_set(29) && present.is_bit_set(30) { + return Err(Error::InvalidFormat); + } + // Need to move to radiotap namespace if present.is_bit_set(29) { present_count = 0; @@ -193,8 +190,6 @@ impl Field for Header { } else if present.is_bit_set(30) { present_count = 0; vendor_namespace = true; - // We'll figure out what namespace it is later, just use none - kinds.push(Kind::VendorNamespace(None)) // Need to stay in the same namespace } else { @@ -225,6 +220,9 @@ pub struct VendorNamespace { impl Field for VendorNamespace { fn from_bytes(input: &[u8]) -> Result { + if input.len() < 6 { + return Err(Error::InvalidLength); + } let mut cursor = Cursor::new(input); let mut oui = [0; 3]; cursor.read_exact(&mut oui)?; diff --git a/src/lib.rs b/src/lib.rs index e19760b..dc30290 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,6 +166,9 @@ impl<'a> Iterator for RadiotapIteratorIntoIter<'a> { Ok(vns) => { start += kind.size(); end += vns.skip_length as usize; + if end > self.cursor.get_ref().len() { + return Some(Err(Error::IncompleteError)); + } kind = Kind::VendorNamespace(Some(vns)); } Err(e) => return Some(Err(e)),