fix(proto): reject invalid echo packet lengths
This commit is contained in:
1 parent
d53dd7f406
commit
dfdc43292a
5 files changed
+68
-6
No files matched your search
@@ -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(
|
||||
|
||||
@@ -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")]
|
||||
|
||||
@@ -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<usize> {
|
||||
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));
|
||||
}
|
||||
}
|
||||
@@ -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};
|
||||
|
||||
|
||||
@@ -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];
|
||||
|
||||
Reference in new issue
Block a user