From d8fe7ee7e6d8f2b9549fd8d00a266c1572c01a0c Mon Sep 17 00:00:00 2001 From: BiagioFesta <15035284+BiagioFesta@users.noreply.github.com> Date: Mon, 2 Oct 2023 21:38:26 +0200 Subject: [PATCH] frame-proto: max payload len on frame read --- wtransport-proto/src/error.rs | 6 +++++ wtransport-proto/src/frame.rs | 47 ++++++++++++++++++++++++++++++++++ wtransport-proto/src/stream.rs | 24 +++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/wtransport-proto/src/error.rs b/wtransport-proto/src/error.rs index 3cde2a8..2f3f568 100644 --- a/wtransport-proto/src/error.rs +++ b/wtransport-proto/src/error.rs @@ -24,6 +24,9 @@ pub enum ErrorCode { /// H3_FRAME_ERROR. Frame, + /// H3_EXCESSIVE_LOAD. + ExcessiveLoad, + /// H3_ID_ERROR. Id, @@ -59,6 +62,7 @@ impl ErrorCode { ErrorCode::ClosedCriticalStream => h3_error_codes::H3_CLOSED_CRITICAL_STREAM, ErrorCode::FrameUnexpected => h3_error_codes::H3_FRAME_UNEXPECTED, ErrorCode::Frame => h3_error_codes::H3_FRAME_ERROR, + ErrorCode::ExcessiveLoad => h3_error_codes::H3_EXCESSIVE_LOAD, ErrorCode::Id => h3_error_codes::H3_ID_ERROR, ErrorCode::Settings => h3_error_codes::H3_SETTINGS_ERROR, ErrorCode::MissingSettings => h3_error_codes::H3_MISSING_SETTINGS, @@ -88,6 +92,7 @@ impl Display for ErrorCode { ErrorCode::ClosedCriticalStream => write!(f, "ClosedCriticalStreamError"), ErrorCode::FrameUnexpected => write!(f, "FrameUnexpectedError"), ErrorCode::Frame => write!(f, "FrameError"), + ErrorCode::ExcessiveLoad => write!(f, "ExcessiveLoad"), ErrorCode::Id => write!(f, "IdError"), ErrorCode::Settings => write!(f, "SettingsError"), ErrorCode::MissingSettings => write!(f, "MissingSettingsError"), @@ -111,6 +116,7 @@ mod h3_error_codes { pub const H3_CLOSED_CRITICAL_STREAM: VarInt = VarInt::from_u32(0x0104); pub const H3_FRAME_UNEXPECTED: VarInt = VarInt::from_u32(0x0105); pub const H3_FRAME_ERROR: VarInt = VarInt::from_u32(0x0106); + pub const H3_EXCESSIVE_LOAD: VarInt = VarInt::from_u32(0x0107); pub const H3_ID_ERROR: VarInt = VarInt::from_u32(0x0108); pub const H3_SETTINGS_ERROR: VarInt = VarInt::from_u32(0x0109); pub const H3_MISSING_SETTINGS: VarInt = VarInt::from_u32(0x010a); diff --git a/wtransport-proto/src/frame.rs b/wtransport-proto/src/frame.rs index d819e4a..e5a7c65 100644 --- a/wtransport-proto/src/frame.rs +++ b/wtransport-proto/src/frame.rs @@ -25,6 +25,9 @@ pub enum ParseError { /// Error for invalid session ID. InvalidSessionId, + + /// Payload required too big. + PayloadTooBig, } /// An error during frame I/O read operation. @@ -107,6 +110,8 @@ pub struct Frame<'a> { } impl<'a> Frame<'a> { + const MAX_PARSE_PAYLOAD_ALLOWED: usize = 4096; + /// Creates a new frame of type [`FrameKind::Headers`]. /// /// # Panics @@ -174,6 +179,11 @@ impl<'a> Frame<'a> { Some(Ok(Self::new_webtransport(session_id))) } else { let payload_len = bytes_reader.get_varint()?.into_inner() as usize; + + if payload_len > Self::MAX_PARSE_PAYLOAD_ALLOWED { + return Some(Err(ParseError::PayloadTooBig)); + } + let payload = bytes_reader.get_bytes(payload_len)?; Some(Ok(Self::new(kind, Cow::Borrowed(payload), None))) @@ -210,6 +220,11 @@ impl<'a> Frame<'a> { _ => e, })? .into_inner() as usize; + + if payload_len > Self::MAX_PARSE_PAYLOAD_ALLOWED { + return Err(IoReadError::Parse(ParseError::PayloadTooBig)); + } + let mut payload = vec![0; payload_len]; reader.get_buffer(&mut payload).await.map_err(|e| match e { @@ -615,6 +630,38 @@ mod tests { )); } + #[test] + fn payload_too_big() { + let mut buffer = Vec::new(); + buffer.put_varint(FrameKind::Data.id()).unwrap(); + buffer + .put_varint(VarInt::from_u32( + Frame::MAX_PARSE_PAYLOAD_ALLOWED as u32 + 1, + )) + .unwrap(); + + assert!(matches!( + Frame::read_from_buffer(&mut BufferReader::new(&buffer)), + Some(Err(ParseError::PayloadTooBig)) + )); + } + + #[tokio::test] + async fn payload_too_big_async() { + let mut buffer = Vec::new(); + buffer.put_varint(FrameKind::Data.id()).unwrap(); + buffer + .put_varint(VarInt::from_u32( + Frame::MAX_PARSE_PAYLOAD_ALLOWED as u32 + 1, + )) + .unwrap(); + + assert!(matches!( + Frame::read_async(&mut &*buffer).await, + Err(IoReadError::Parse(ParseError::PayloadTooBig)), + )); + } + mod utils { use super::*; diff --git a/wtransport-proto/src/stream.rs b/wtransport-proto/src/stream.rs index 1667b02..167bb2a 100644 --- a/wtransport-proto/src/stream.rs +++ b/wtransport-proto/src/stream.rs @@ -97,6 +97,9 @@ pub mod biremote { Err(frame::ParseError::InvalidSessionId) => { return Some(Err(ErrorCode::Id)); } + Err(frame::ParseError::PayloadTooBig) => { + return Some(Err(ErrorCode::ExcessiveLoad)); + } } } } @@ -122,6 +125,9 @@ pub mod biremote { Err(frame::IoReadError::Parse(frame::ParseError::InvalidSessionId)) => { return Err(IoReadError::H3(ErrorCode::Id)); } + Err(frame::IoReadError::Parse(frame::ParseError::PayloadTooBig)) => { + return Err(IoReadError::H3(ErrorCode::ExcessiveLoad)); + } Err(frame::IoReadError::IO(io_error)) => { if matches!(io_error, bytes::IoReadError::UnexpectedFin) { return Err(IoReadError::H3(ErrorCode::Frame)); @@ -279,6 +285,9 @@ pub mod bilocal { Err(frame::ParseError::InvalidSessionId) => { return Some(Err(ErrorCode::Id)); } + Err(frame::ParseError::PayloadTooBig) => { + return Some(Err(ErrorCode::ExcessiveLoad)); + } } } } @@ -304,6 +313,9 @@ pub mod bilocal { Err(frame::IoReadError::Parse(frame::ParseError::InvalidSessionId)) => { return Err(IoReadError::H3(ErrorCode::Id)); } + Err(frame::IoReadError::Parse(frame::ParseError::PayloadTooBig)) => { + return Err(IoReadError::H3(ErrorCode::ExcessiveLoad)); + } Err(frame::IoReadError::IO(io_error)) => { if matches!(io_error, bytes::IoReadError::UnexpectedFin) { return Err(IoReadError::H3(ErrorCode::Frame)); @@ -590,6 +602,9 @@ pub mod uniremote { Err(frame::ParseError::InvalidSessionId) => { return Some(Err(ErrorCode::Id)); } + Err(frame::ParseError::PayloadTooBig) => { + return Some(Err(ErrorCode::ExcessiveLoad)); + } } } } @@ -621,6 +636,9 @@ pub mod uniremote { Err(frame::IoReadError::Parse(frame::ParseError::InvalidSessionId)) => { return Err(IoReadError::H3(ErrorCode::Id)); } + Err(frame::IoReadError::Parse(frame::ParseError::PayloadTooBig)) => { + return Err(IoReadError::H3(ErrorCode::ExcessiveLoad)); + } Err(frame::IoReadError::IO(io_error)) => { if matches!(io_error, bytes::IoReadError::UnexpectedFin) { return Err(IoReadError::H3(ErrorCode::Frame)); @@ -913,6 +931,9 @@ pub mod session { Err(frame::ParseError::InvalidSessionId) => { return Some(Err(ErrorCode::Id)); } + Err(frame::ParseError::PayloadTooBig) => { + return Some(Err(ErrorCode::ExcessiveLoad)); + } } } } @@ -938,6 +959,9 @@ pub mod session { Err(frame::IoReadError::Parse(frame::ParseError::InvalidSessionId)) => { return Err(IoReadError::H3(ErrorCode::Id)); } + Err(frame::IoReadError::Parse(frame::ParseError::PayloadTooBig)) => { + return Err(IoReadError::H3(ErrorCode::ExcessiveLoad)); + } Err(frame::IoReadError::IO(io_error)) => { if matches!(io_error, bytes::IoReadError::UnexpectedFin) { return Err(IoReadError::H3(ErrorCode::Frame));