Skip to content

Commit

Permalink
Avoid panic in RouteNextHopBuffer length checks
Browse files Browse the repository at this point in the history
If the next-hops length is u16::MAX, adding 8 (the PAYLOAD_OFFSET) to
it would cause integer overflow and lead to a panic. Rather than
switching to `saturating_add`, remove the addition entirely. It was
unnecessary, as the provided length already accounts for the RTA struct.
  • Loading branch information
Jeff-A-Martin authored and cathay4t committed Aug 12, 2024
1 parent fceb9c2 commit 666edbc
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/route/next_hops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl<T: AsRef<[u8]>> RouteNextHopBuffer<T> {
return Err(format!(
"invalid RouteNextHopBuffer: length {} < {}",
len,
8 + self.length()
self.length(),
)
.into());
}
Expand Down
14 changes: 13 additions & 1 deletion src/route/tests/route_flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use netlink_packet_utils::traits::{Emitable, Parseable};
use crate::route::flags::RouteFlags;
use crate::route::{
RouteAttribute, RouteHeader, RouteMessage, RouteMessageBuffer,
RouteProtocol, RouteScope, RouteType,
RouteNextHopBuffer, RouteProtocol, RouteScope, RouteType,
};
use crate::AddressFamily;

Expand Down Expand Up @@ -54,3 +54,15 @@ fn test_ipv6_add_route_onlink() {

assert_eq!(buf, raw);
}

// Verify that [`RouteNextHopBuffer`] rejects the buffer when provided with
// an invalid length.
#[test]
fn test_next_hop_max_buffer_len() {
// Route next-hop buffer layout:
// |byte0|byte1|byte2|byte3|byte4|byte5|byte6|byte7|bytes8+|
// |-----|-----|-----|-----|-----|-----|-----|-----|-------|
// | length |flags|hops | Interface Index |Payload|
let buffer = [0xff, 0xff, 0, 0, 0, 0, 0, 0];
assert!(RouteNextHopBuffer::new_checked(buffer).is_err());
}

0 comments on commit 666edbc

Please sign in to comment.