From 91819bf25e6f6c1fa22936b75a6f600f657a05af Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 29 May 2019 14:40:28 -0700 Subject: [PATCH] check for overly large header field in send_headers --- src/codec/error.rs | 4 +++ src/frame/headers.rs | 48 ++++++++++++++++++++++++++++++++++++ src/hpack/mod.rs | 2 +- src/proto/streams/send.rs | 8 ++++++ src/proto/streams/streams.rs | 3 ++- 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/codec/error.rs b/src/codec/error.rs index 45b4251..12f90eb 100644 --- a/src/codec/error.rs +++ b/src/codec/error.rs @@ -35,6 +35,9 @@ pub enum UserError { /// The payload size is too big PayloadTooBig, + /// A header size is too big + HeaderTooBig, + /// The application attempted to initiate too many streams to remote. Rejected, @@ -131,6 +134,7 @@ impl error::Error for UserError { InactiveStreamId => "inactive stream", UnexpectedFrameType => "unexpected frame type", PayloadTooBig => "payload too big", + HeaderTooBig => "header too big", Rejected => "rejected", ReleaseCapacityTooBig => "release capacity too big", OverflowedStreamId => "stream ID overflowed", diff --git a/src/frame/headers.rs b/src/frame/headers.rs index d4f54bf..d5215a0 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -12,6 +12,10 @@ use string::String; use std::fmt; use std::io::Cursor; +// Minimum MAX_FRAME_SIZE is 16kb, so save some arbitrary space for frame +// head and other header bits. +const MAX_HEADER_LENGTH: usize = 1024 * 16 - 100; + /// Header frame /// /// This could be either a request or a response. @@ -232,6 +236,10 @@ impl Headers { self.header_block.is_over_size } + pub(crate) fn has_too_big_field(&self) -> bool { + self.header_block.has_too_big_field() + } + pub fn into_parts(self) -> (Pseudo, HeaderMap) { (self.header_block.pseudo, self.header_block.fields) } @@ -844,6 +852,46 @@ impl HeaderBlock { .map(|(name, value)| decoded_header_size(name.as_str().len(), value.len())) .sum::() } + + /// Iterate over all pseudos and headers to see if any individual pair + /// would be too large to encode. + pub(crate) fn has_too_big_field(&self) -> bool { + macro_rules! pseudo_size { + ($name:ident) => ({ + self.pseudo + .$name + .as_ref() + .map(|m| decoded_header_size(stringify!($name).len() + 1, m.as_str().len())) + .unwrap_or(0) + }); + } + + if pseudo_size!(method) > MAX_HEADER_LENGTH { + return true; + } + + if pseudo_size!(scheme) > MAX_HEADER_LENGTH { + return true; + } + + if pseudo_size!(authority) > MAX_HEADER_LENGTH { + return true; + } + + if pseudo_size!(path) > MAX_HEADER_LENGTH { + return true; + } + + // skip :status, its never going to be too big + + for (name, value) in &self.fields { + if decoded_header_size(name.as_str().len(), value.len()) > MAX_HEADER_LENGTH { + return true; + } + } + + false + } } fn decoded_header_size(name: usize, value: usize) -> usize { diff --git a/src/hpack/mod.rs b/src/hpack/mod.rs index 956de88..71e7474 100644 --- a/src/hpack/mod.rs +++ b/src/hpack/mod.rs @@ -1,6 +1,6 @@ mod encoder; mod decoder; -mod header; +pub(crate) mod header; mod huffman; mod table; diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 941a7f4..9ab2a88 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -85,6 +85,10 @@ impl Send { } } + if frame.has_too_big_field() { + return Err(UserError::HeaderTooBig); + } + let end_stream = frame.is_end_stream(); // Update the state @@ -216,6 +220,10 @@ impl Send { return Err(UserError::UnexpectedFrameType); } + if frame.has_too_big_field() { + return Err(UserError::HeaderTooBig); + } + stream.state.send_close(); trace!("send_trailers -- queuing; frame={:?}", frame); diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 3b980b1..b0453bc 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -180,8 +180,9 @@ where Ok(()) => Ok(()), Err(RecvHeaderBlockError::Oversize(resp)) => { if let Some(resp) = resp { - let _ = actions.send.send_headers( + let sent = actions.send.send_headers( resp, send_buffer, stream, counts, &mut actions.task); + debug_assert!(sent.is_ok(), "oversize response should not fail"); actions.send.schedule_implicit_reset( stream,