diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 3b6a9bd..f843c69 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -153,7 +153,9 @@ impl Headers { // Read the padding length if flags.is_padded() { - // TODO: Ensure payload is sized correctly + if src.len() < 1 { + return Err(Error::MalformedMessage); + } pad = src[0] as usize; // Drop the padding @@ -162,6 +164,9 @@ impl Headers { // Read the stream dependency let stream_dep = if flags.is_priority() { + if src.len() < 5 { + return Err(Error::MalformedMessage); + } let stream_dep = StreamDependency::load(&src[..5])?; if stream_dep.dependency_id() == head.stream_id() { @@ -290,6 +295,10 @@ impl PushPromise { // Read the padding length if flags.is_padded() { + if src.len() < 1 { + return Err(Error::MalformedMessage); + } + // TODO: Ensure payload is sized correctly pad = src[0] as usize; @@ -297,6 +306,10 @@ impl PushPromise { let _ = src.split_to(1); } + if src.len() < 5 { + return Err(Error::MalformedMessage); + } + let (promised_id, _) = StreamId::parse(&src[..4]); // Drop promised_id bytes let _ = src.split_to(5); diff --git a/src/hpack/header.rs b/src/hpack/header.rs index b91701f..9034a03 100644 --- a/src/hpack/header.rs +++ b/src/hpack/header.rs @@ -1,4 +1,4 @@ -use super::DecoderError; +use super::{DecoderError, NeedMore}; use bytes::Bytes; use http::{Method, StatusCode}; @@ -60,6 +60,9 @@ impl Header> { impl Header { pub fn new(name: Bytes, value: Bytes) -> Result { + if name.len() == 0 { + return Err(DecoderError::NeedMore(NeedMore::UnexpectedEndOfStream)); + } if name[0] == b':' { match &name[1..] { b"authority" => { diff --git a/src/proto/connection.rs b/src/proto/connection.rs index c1b5ee1..55c4d85 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -315,7 +315,7 @@ where // but should allow continuing to process current streams // until they are all EOS. Once they are, State should // transition to GoAway. - self.streams.recv_go_away(&frame); + self.streams.recv_go_away(&frame)?; self.error = Some(frame.reason()); }, Some(Ping(frame)) => { diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index d18eaaf..3ead21c 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -310,7 +310,7 @@ where last_processed_id } - pub fn recv_go_away(&mut self, frame: &frame::GoAway) { + pub fn recv_go_away(&mut self, frame: &frame::GoAway) -> Result<(), RecvError> { let mut me = self.inner.lock().unwrap(); let me = &mut *me; @@ -322,6 +322,17 @@ where let last_stream_id = frame.last_stream_id(); let err = frame.reason().into(); + if last_stream_id > actions.recv.max_stream_id() { + // The remote endpoint sent a `GOAWAY` frame indicating a stream + // that we never sent, or that we have already terminated on account + // of previous `GOAWAY` frame. In either case, that is illegal. + // (When sending multiple `GOAWAY`s, "Endpoints MUST NOT increase + // the value they send in the last stream identifier, since the + // peers might already have retried unprocessed requests on another + // connection.") + return Err(RecvError::Connection(Reason::PROTOCOL_ERROR)); + } + actions.recv.go_away(last_stream_id); me.store @@ -337,6 +348,8 @@ where .unwrap(); actions.conn_error = Some(err); + + Ok(()) } pub fn recv_eof(&mut self) {