From 409a19df2801c92a87c843f15bbb8e7b2ef8f959 Mon Sep 17 00:00:00 2001 From: Lee Smet Date: Thu, 18 Jan 2024 15:04:56 +0100 Subject: [PATCH] Close #100: Harden prefix decoding Prefix decoding copies based on the earlier specified plen. For IPv4 and IPv6, these have an implied maximum of 32 and 128, respectively. Since that is not checked, a remote node could supply a value which is too high, which in turn cuases the copy to read too many bytes, or to copy outside of the destination bound. Both of these scenarios result in a panic. That panic would only affect the connection of that remote peer and leave the remainder of the system intact, so no real harm is done. Now, we cleanly return a None value. We don't attempt to skip bytes to leave the buffer in a known good state, and most likely a following read will find a bad frame. Ultimately, this would lead to the connection dying due to no IHU's being received. Signed-off-by: Lee Smet --- CHANGELOG.md | 4 ++++ src/babel/route_request.rs | 9 +++++++++ src/babel/seqno_request.rs | 16 ++++++++++++++-- src/babel/update.rs | 20 ++++++++++++++++---- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f84b2b3..bd612a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Prefix decoding is now more resilient to bad prefix lenghts. + ## [0.3.0] - 2024-01-17 ### Added diff --git a/src/babel/route_request.rs b/src/babel/route_request.rs index 91ff515..e89ebaa 100644 --- a/src/babel/route_request.rs +++ b/src/babel/route_request.rs @@ -56,18 +56,27 @@ impl RouteRequest { let prefix_ip = match ae { AE_WILDCARD => None, AE_IPV4 => { + if plen > 32 { + return None; + } let mut raw_ip = [0; 4]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Some(Ipv4Addr::from(raw_ip).into()) } AE_IPV6 => { + if plen > 128 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Some(Ipv6Addr::from(raw_ip).into()) } AE_IPV6_LL => { + if plen != 64 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[0] = 0xfe; raw_ip[1] = 0x80; diff --git a/src/babel/seqno_request.rs b/src/babel/seqno_request.rs index 4160a86..3446cc5 100644 --- a/src/babel/seqno_request.rs +++ b/src/babel/seqno_request.rs @@ -105,22 +105,34 @@ impl SeqNoRequest { let prefix = match ae { AE_WILDCARD => { + if plen != 0 { + return None; + } // TODO: this is a temporary placeholder until we figure out how to handle this Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0).into() } AE_IPV4 => { + if plen > 32 { + return None; + } let mut raw_ip = [0; 4]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Ipv4Addr::from(raw_ip).into() } AE_IPV6 => { + if plen > 128 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Ipv6Addr::from(raw_ip).into() } AE_IPV6_LL => { + if plen != 64 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[0] = 0xfe; raw_ip[1] = 0x80; @@ -257,7 +269,7 @@ mod tests { let mut buf = bytes::BytesMut::from( &[ - 3, 92, 0, 42, 232, 0, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, + 3, 64, 0, 42, 232, 0, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 0, 10, 0, 20, 0, 30, 0, 40, ][..], @@ -266,7 +278,7 @@ mod tests { let snr = super::SeqNoRequest { seqno: 42.into(), hop_count: NonZeroU8::new(232).unwrap(), - prefix: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 92) + prefix: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 64) .expect("92 is a valid IPv6 prefix size; qed"), router_id: RouterId::from([4u8; RouterId::BYTE_SIZE]), }; diff --git a/src/babel/update.rs b/src/babel/update.rs index 8ab94a3..4256c11 100644 --- a/src/babel/update.rs +++ b/src/babel/update.rs @@ -104,22 +104,34 @@ impl Update { let prefix_size = ((plen + 7) / 8) as usize; let prefix = match ae { AE_WILDCARD => { + if prefix_size != 0 { + return None; + } // TODO: this is a temporary placeholder until we figure out how to handle this Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0).into() } AE_IPV4 => { + if plen > 32 { + return None; + } let mut raw_ip = [0; 4]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Ipv4Addr::from(raw_ip).into() } AE_IPV6 => { + if plen > 128 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[..prefix_size].copy_from_slice(&src[..prefix_size]); src.advance(prefix_size); Ipv6Addr::from(raw_ip).into() } AE_IPV6_LL => { + if plen != 64 { + return None; + } let mut raw_ip = [0; 16]; raw_ip[0] = 0xfe; raw_ip[1] = 0x80; @@ -262,7 +274,7 @@ mod tests { let mut buf = bytes::BytesMut::from( &[ - 3, 0, 92, 0, 3, 232, 0, 42, 3, 1, 0, 10, 0, 20, 0, 30, 0, 40, 4, 4, 4, 4, 4, 4, 4, + 3, 0, 64, 0, 3, 232, 0, 42, 3, 1, 0, 10, 0, 20, 0, 30, 0, 40, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, ][..], @@ -273,7 +285,7 @@ mod tests { interval: 1000, seqno: 42.into(), metric: 769.into(), - subnet: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 92) + subnet: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 64) .expect("92 is a valid IPv6 prefix size; qed"), router_id: RouterId::from([4u8; RouterId::BYTE_SIZE]), }; @@ -311,7 +323,7 @@ mod tests { // Set all flag bits, only allowed bits should be set on the decoded value let mut buf = bytes::BytesMut::from( &[ - 3, 255, 92, 0, 3, 232, 0, 42, 3, 1, 0, 10, 0, 20, 0, 30, 0, 40, 4, 4, 4, 4, 4, 4, + 3, 255, 64, 0, 3, 232, 0, 42, 3, 1, 0, 10, 0, 20, 0, 30, 0, 40, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, ][..], @@ -322,7 +334,7 @@ mod tests { interval: 1000, seqno: 42.into(), metric: 769.into(), - subnet: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 92) + subnet: Subnet::new(Ipv6Addr::new(0xfe80, 0, 0, 0, 10, 20, 30, 40).into(), 64) .expect("92 is a valid IPv6 prefix size; qed"), router_id: RouterId::from([4u8; RouterId::BYTE_SIZE]), };