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 <lee.smet@hotmail.com>
This commit is contained in:
Lee Smet
2024-01-18 15:04:56 +01:00
parent 002563cc82
commit 409a19df28
4 changed files with 43 additions and 6 deletions
+4
View File
@@ -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
+9
View File
@@ -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;
+14 -2
View File
@@ -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]),
};
+16 -4
View File
@@ -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]),
};