Skip to content

Commit

Permalink
bgp: validate message length in unit tests
Browse files Browse the repository at this point in the history
In normal operation, `Message::get_message_len()` is used to
ensure the input buffer contains a complete BGP message before
decoding. Apply the same validation in unit tests to avoid passing
malformed data to the decoding function, which assumes the message
length is valid.

Additionally, simplify the logic for determining the presence of
the NLRI field to improve readability.

Fixes #44

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
  • Loading branch information
rwestphal committed Jan 16, 2025
1 parent cbc3929 commit 9b3acda
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
3 changes: 1 addition & 2 deletions holo-bgp/src/packet/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,7 @@ impl UpdateMsg {
// Path Attributes.
if attr_len != 0 {
let mut buf_attr = buf.copy_to_bytes(attr_len as usize);
let nlri_present =
(msg_len - Self::MIN_LEN - wdraw_len - attr_len) > 0;
let nlri_present = buf.remaining() > 0;
attrs = Attrs::decode(
&mut buf_attr,
cxt,
Expand Down
4 changes: 3 additions & 1 deletion holo-bgp/tests/packet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ fn test_decode_msg(bytes: &[u8], msg_expected: &Message) {
capabilities: [NegotiatedCapability::FourOctetAsNumber].into(),
};

let msg_actual = Message::decode(&bytes, &cxt).unwrap();
let msg_size = Message::get_message_len(&bytes)
.expect("Buffer doesn't contain a full BGP message");
let msg_actual = Message::decode(&bytes[0..msg_size], &cxt).unwrap();
assert_eq!(*msg_expected, msg_actual);
}
14 changes: 13 additions & 1 deletion holo-bgp/tests/packet/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,19 @@ fn test_decode_malformed_updates() {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00,
],
// Malformed message length.
vec![
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x1b, 0x02, 0x00, 0x00, 0x00,
0x2d, 0x00, 0x00, 0x00, 0xf5, 0x00, 0x00, 0x00, 0x00, 0x14, 0x9f,
0x49, 0x00, 0x00, 0xff, 0x00, 0x00, 0x00, 0x00, 0x14, 0x9f, 0x49,
0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, 0x00, 0x00, 0x00, 0xf5, 0x00,
0x00, 0x00, 0x00, 0x14, 0x9f, 0x49, 0x00, 0x00, 0xff, 0x00, 0x00,
0x00, 0x00, 0x14, 0x9f, 0x49, 0x00, 0x00,
],
] {
let _ = Message::decode(bytes, &cxt);
if let Some(msg_size) = Message::get_message_len(bytes) {
let _ = Message::decode(&bytes[0..msg_size], &cxt);
}
}
}

0 comments on commit 9b3acda

Please sign in to comment.