Skip to content

Commit

Permalink
frame-proto: max payload len on frame read
Browse files Browse the repository at this point in the history
  • Loading branch information
@BiagioFesta
BiagioFesta committed Oct 2, 2023
1 parent fcd63d6 commit d8fe7ee
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
6 changes: 6 additions & 0 deletions wtransport-proto/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ pub enum ErrorCode {
/// H3_FRAME_ERROR.
Frame,

/// H3_EXCESSIVE_LOAD.
ExcessiveLoad,

/// H3_ID_ERROR.
Id,

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand All @@ -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);
Expand Down
47 changes: 47 additions & 0 deletions wtransport-proto/src/frame.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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::*;

Expand Down
24 changes: 24 additions & 0 deletions wtransport-proto/src/stream.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
}
Expand All @@ -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));
Expand Down Expand Up @@ -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));
}
}
}
}
Expand All @@ -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));
Expand Down Expand Up @@ -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));
}
}
}
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
}
}
}
}
Expand All @@ -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));
Expand Down

0 comments on commit d8fe7ee

Please sign in to comment.