From fbb345cdf634a9ce7e9cebd319275bb251a9d105 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Fri, 8 May 2026 09:58:31 +0700 Subject: [PATCH] fix(proto): reject invalid echo packet lengths --- crates/irtt-proto/src/echo.rs | 15 +++++++++++++-- crates/irtt-proto/src/error.rs | 4 ++++ crates/irtt-proto/src/layout.rs | 34 +++++++++++++++++++++++++++++++-- crates/irtt-proto/src/lib.rs | 2 +- crates/irtt-proto/src/params.rs | 19 +++++++++++++++++- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/crates/irtt-proto/src/echo.rs b/crates/irtt-proto/src/echo.rs index 0e4dfc6..5de3165 100644 --- a/crates/irtt-proto/src/echo.rs +++ b/crates/irtt-proto/src/echo.rs @@ -1,7 +1,7 @@ use crate::{ flags::{has, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY}, hmac, - layout::{echo_packet_len, PacketLayout}, + layout::{try_echo_packet_len, PacketLayout}, open::{check_hmac_presence, reject}, params::Params, validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE, @@ -43,7 +43,7 @@ pub fn encode_echo_request(request: &EchoRequest, hmac_key: Option<&[u8]>) -> Re flags |= FLAG_HMAC; } let layout = PacketLayout::echo(hmac_key.is_some(), &request.params); - let len = echo_packet_len(hmac_key.is_some(), &request.params); + let len = try_echo_packet_len(hmac_key.is_some(), &request.params)?; let payload_offset = layout.header_len(); let available_payload_len = len.saturating_sub(payload_offset); if request.payload.len() > available_payload_len { @@ -231,6 +231,17 @@ mod tests { assert_eq!(request, original); } + #[test] + fn echo_request_rejects_negative_requested_length() { + let mut request = echo_request_with_payload(0, Vec::new()); + request.params.length = -1; + + assert_eq!( + encode_echo_request(&request, None), + Err(ProtoError::NegativePacketLength { length: -1 }) + ); + } + #[test] fn hmac_echo_request_places_token_and_sequence_after_hmac() { let packet = encode_echo_request( diff --git a/crates/irtt-proto/src/error.rs b/crates/irtt-proto/src/error.rs index 6fdb5ff..81fdd9e 100644 --- a/crates/irtt-proto/src/error.rs +++ b/crates/irtt-proto/src/error.rs @@ -28,6 +28,10 @@ pub enum ProtoError { InvalidUtf8, #[error("invalid enum value {value} for {name}")] InvalidEnum { name: &'static str, value: i64 }, + #[error("packet length must be non-negative, got {length}")] + NegativePacketLength { length: i64 }, + #[error("parameter {tag} length is too large: {length} bytes")] + ParameterLengthTooLarge { tag: u64, length: u64 }, #[error("open reply has zero token without close flag")] ZeroToken, #[error("trailing or malformed parameter payload")] diff --git a/crates/irtt-proto/src/layout.rs b/crates/irtt-proto/src/layout.rs index 6604be9..cd56466 100644 --- a/crates/irtt-proto/src/layout.rs +++ b/crates/irtt-proto/src/layout.rs @@ -1,7 +1,7 @@ use crate::{ params::{Params, StampAt}, - HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE, RECV_WINDOW_SIZE, SEQ_SIZE, TIMESTAMP_SIZE, - TOKEN_SIZE, + ProtoError, Result, HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE, RECV_WINDOW_SIZE, SEQ_SIZE, + TIMESTAMP_SIZE, TOKEN_SIZE, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -117,6 +117,15 @@ pub fn echo_packet_len(hmac: bool, params: &Params) -> usize { header_len.max(requested) } +pub fn try_echo_packet_len(hmac: bool, params: &Params) -> Result { + let header_len = echo_header_len(hmac, params); + let requested = + usize::try_from(params.length).map_err(|_| ProtoError::NegativePacketLength { + length: params.length, + })?; + Ok(header_len.max(requested)) +} + #[cfg(test)] mod tests { use super::*; @@ -295,4 +304,25 @@ mod tests { p.length = 92; assert_eq!(echo_packet_len(false, &p), 92); } + + #[test] + fn checked_packet_len_rejects_negative_requested_length() { + let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both); + p.length = -1; + + assert_eq!( + try_echo_packet_len(false, &p), + Err(ProtoError::NegativePacketLength { length: -1 }) + ); + } + + #[test] + fn checked_packet_len_preserves_non_negative_header_floor() { + let mut p = params(ReceivedStats::Both, StampAt::Both, Clock::Both); + p.length = 0; + assert_eq!(try_echo_packet_len(false, &p), Ok(60)); + + p.length = 92; + assert_eq!(try_echo_packet_len(false, &p), Ok(92)); + } } diff --git a/crates/irtt-proto/src/lib.rs b/crates/irtt-proto/src/lib.rs index 36311a9..834b0ef 100644 --- a/crates/irtt-proto/src/lib.rs +++ b/crates/irtt-proto/src/lib.rs @@ -15,7 +15,7 @@ pub use echo::{decode_echo_reply, encode_echo_request, EchoReply, EchoRequest, T pub use error::{ProtoError, Result}; 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 layout::{echo_header_len, echo_packet_len, try_echo_packet_len, PacketLayout}; pub use open::{decode_open_reply, encode_open_request, OpenReply, OpenRequest}; pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt}; diff --git a/crates/irtt-proto/src/params.rs b/crates/irtt-proto/src/params.rs index 2a51be9..06d1f8a 100644 --- a/crates/irtt-proto/src/params.rs +++ b/crates/irtt-proto/src/params.rs @@ -57,7 +57,8 @@ impl Params { 9 => { let (len, used) = varint::decode_uvarint(&input[pos..])?; pos += used; - let len = len as usize; + let len = usize::try_from(len) + .map_err(|_| ProtoError::ParameterLengthTooLarge { tag, length: len })?; if input.len().saturating_sub(pos) < len { return Err(ProtoError::MalformedParams); } @@ -406,6 +407,22 @@ mod tests { assert_eq!(Params::decode(&encoded), Err(ProtoError::InvalidUtf8)); } + #[cfg(target_pointer_width = "32")] + #[test] + fn server_fill_length_too_large_for_usize_is_rejected() { + let mut encoded = Vec::new(); + varint::encode_uvarint(9, &mut encoded); + varint::encode_uvarint(u64::from(u32::MAX) + 1, &mut encoded); + + assert_eq!( + Params::decode(&encoded), + Err(ProtoError::ParameterLengthTooLarge { + tag: 9, + length: u64::from(u32::MAX) + 1, + }) + ); + } + #[test] fn truncated_varint_parameter_is_rejected() { let encoded = [1, 0x80];