feat(proto): enforce maximum length for server fill parameter
Move MAX_SERVER_FILL_BYTES to irtt-proto and validate it during decoding to prevent processing oversized parameters.
This commit is contained in:
1 parent
2d6581f31f
commit
64da60d480
3 files changed
+41
-2
No files matched your search
@@ -17,7 +17,7 @@ pub use flags::*;
|
||||
pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac};
|
||||
pub use layout::{echo_header_len, echo_packet_len, PacketLayout};
|
||||
pub use open::{decode_open_reply, encode_open_request, OpenReply, OpenRequest};
|
||||
pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt};
|
||||
pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt, MAX_SERVER_FILL_BYTES};
|
||||
|
||||
pub const MAGIC: [u8; 3] = [0x14, 0xA7, 0x5B];
|
||||
pub const PROTOCOL_VERSION: i64 = 1;
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use crate::{varint, ProtoError, Result, PROTOCOL_VERSION};
|
||||
|
||||
pub const MAX_SERVER_FILL_BYTES: usize = 32;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub struct Params {
|
||||
pub protocol_version: i64,
|
||||
@@ -59,6 +61,12 @@ impl Params {
|
||||
pos += used;
|
||||
let len = usize::try_from(len)
|
||||
.map_err(|_| ProtoError::ParameterLengthTooLarge { tag, length: len })?;
|
||||
if len > MAX_SERVER_FILL_BYTES {
|
||||
return Err(ProtoError::ParameterLengthTooLarge {
|
||||
tag,
|
||||
length: len as u64,
|
||||
});
|
||||
}
|
||||
if input.len().saturating_sub(pos) < len {
|
||||
return Err(ProtoError::MalformedParams);
|
||||
}
|
||||
@@ -206,6 +214,14 @@ mod tests {
|
||||
assert_eq!(Params::decode(¶ms.encode()), Ok(params));
|
||||
}
|
||||
|
||||
fn encode_server_fill_value(value: &[u8]) -> Vec<u8> {
|
||||
let mut encoded = Vec::new();
|
||||
varint::encode_uvarint(9, &mut encoded);
|
||||
varint::encode_uvarint(value.len() as u64, &mut encoded);
|
||||
encoded.extend_from_slice(value);
|
||||
encoded
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn params_round_trip() {
|
||||
let params = Params {
|
||||
@@ -323,6 +339,29 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_fill_decode_accepts_max_length() {
|
||||
let value = b"0123456789abcdef0123456789abcdef";
|
||||
let params = Params::decode(&encode_server_fill_value(value)).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
params.server_fill,
|
||||
Some(ServerFill {
|
||||
value: "0123456789abcdef0123456789abcdef".to_owned(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_fill_decode_rejects_oversized_length() {
|
||||
let value = b"0123456789abcdef0123456789abcdefx";
|
||||
|
||||
assert_eq!(
|
||||
Params::decode(&encode_server_fill_value(value)),
|
||||
Err(ProtoError::ParameterLengthTooLarge { tag: 9, length: 33 })
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_fill_tag_and_length_are_encoded_before_utf8_bytes() {
|
||||
let params = Params {
|
||||
|
||||
Reference in new issue
Block a user