From ccf030250098cf914aa93ce52fb4647a235af7d6 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Thu, 18 Apr 2024 03:44:07 +0300 Subject: [PATCH] fix: Check that frame types use minimal varint encoding (#1811) * fix: Check that frame types use minimal varint encoding * Update neqo-transport/src/frame.rs Signed-off-by: Lars Eggert * Update frame.rs Co-authored-by: Max Inden Signed-off-by: Lars Eggert * Address code review * Update neqo-common/src/codec.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-common/src/codec.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Fixups --------- Signed-off-by: Lars Eggert Co-authored-by: Max Inden Co-authored-by: Martin Thomson --- neqo-transport/src/frame.rs | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/neqo-transport/src/frame.rs b/neqo-transport/src/frame.rs index 1317474a20..eba7009d4b 100644 --- a/neqo-transport/src/frame.rs +++ b/neqo-transport/src/frame.rs @@ -8,7 +8,7 @@ use std::ops::RangeInclusive; -use neqo_common::{qtrace, Decoder}; +use neqo_common::{qtrace, Decoder, Encoder}; use crate::{ cid::MAX_CONNECTION_ID_LEN, @@ -464,8 +464,18 @@ impl<'a> Frame<'a> { }) } - // TODO(ekr@rtfm.com): check for minimal encoding + // Check for minimal encoding of frame type. + let pos = dec.offset(); let t = dv(dec)?; + // RFC 9000, Section 12.4: + // + // The Frame Type field uses a variable-length integer encoding [...], + // with one exception. To ensure simple and efficient implementations of + // frame parsing, a frame type MUST use the shortest possible encoding. + if Encoder::varint_len(t) != dec.offset() - pos { + return Err(Error::ProtocolViolation); + } + match t { FRAME_TYPE_PADDING => { let mut length: u16 = 1; @@ -660,7 +670,7 @@ mod tests { fn just_dec(f: &Frame, s: &str) { let encoded = Encoder::from_hex(s); - let decoded = Frame::decode(&mut encoded.as_decoder()).unwrap(); + let decoded = Frame::decode(&mut encoded.as_decoder()).expect("Failed to decode frame"); assert_eq!(*f, decoded); } @@ -1005,14 +1015,14 @@ mod tests { fill: true, }; - just_dec(&f, "4030010203"); + just_dec(&f, "30010203"); // With the length bit. let f = Frame::Datagram { data: &[1, 2, 3], fill: false, }; - just_dec(&f, "403103010203"); + just_dec(&f, "3103010203"); } #[test] @@ -1026,4 +1036,15 @@ mod tests { assert_eq!(Err(Error::TooMuchData), Frame::decode(&mut e.as_decoder())); } + + #[test] + #[should_panic(expected = "Failed to decode frame")] + fn invalid_frame_type_len() { + let f = Frame::Datagram { + data: &[1, 2, 3], + fill: true, + }; + + just_dec(&f, "4030010203"); + } }