From 24a065f5bf465b8dda81fc03fb758dc01feb47cb Mon Sep 17 00:00:00 2001 From: Yuchen Wu Date: Mon, 11 Nov 2024 11:41:36 -0800 Subject: [PATCH] Header frame with non-zero content-length and END_STREAM is malformed Before this change, content-length underflow is only checked when receiving date frames. The underflow error was never triggered if data frames are never received. This change adds similar check for headers frames. --- src/frame/headers.rs | 4 +++ src/proto/streams/recv.rs | 12 +++++++ tests/h2-tests/tests/client_request.rs | 43 ++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 8411a3aa..a0f282e3 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -254,6 +254,10 @@ impl Headers { &mut self.header_block.pseudo } + pub(crate) fn pseudo(&self) -> &Pseudo { + &self.header_block.pseudo + } + /// Whether it has status 1xx pub(crate) fn is_informational(&self) -> bool { self.header_block.pseudo.is_informational() diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs index a70527e2..d8572d00 100644 --- a/src/proto/streams/recv.rs +++ b/src/proto/streams/recv.rs @@ -185,6 +185,18 @@ impl Recv { }; stream.content_length = ContentLength::Remaining(content_length); + // END_STREAM on headers frame with non-zero content-length is malformed. + // https://datatracker.ietf.org/doc/html/rfc9113#section-8.1.1 + if frame.is_end_stream() + && content_length > 0 + && frame + .pseudo() + .status + .map_or(true, |status| status != 204 && status != 304) + { + proto_err!(stream: "recv_headers with END_STREAM: content-length is not zero; stream={:?};", stream.id); + return Err(Error::library_reset(stream.id, Reason::PROTOCOL_ERROR).into()); + } } } diff --git a/tests/h2-tests/tests/client_request.rs b/tests/h2-tests/tests/client_request.rs index e914d484..8c991839 100644 --- a/tests/h2-tests/tests/client_request.rs +++ b/tests/h2-tests/tests/client_request.rs @@ -1351,6 +1351,49 @@ async fn allow_empty_data_for_head() { join(srv, h2).await; } +#[tokio::test] +async fn reject_none_zero_content_length_header_with_end_stream() { + h2_support::trace_init!(); + let (io, mut srv) = mock::new(); + + let srv = async move { + let settings = srv.assert_client_handshake().await; + assert_default_settings!(settings); + srv.recv_frame( + frames::headers(1) + .request("GET", "https://example.com/") + .eos(), + ) + .await; + srv.send_frame( + frames::headers(1) + .response(200) + .field("content-length", 100) + .eos(), + ) + .await; + }; + + let h2 = async move { + let (mut client, h2) = client::Builder::new() + .handshake::<_, Bytes>(io) + .await + .unwrap(); + tokio::spawn(async { + h2.await.expect("connection failed"); + }); + let request = Request::builder() + .method(Method::GET) + .uri("https://example.com/") + .body(()) + .unwrap(); + let (response, _) = client.send_request(request, true).unwrap(); + let _ = response.await.unwrap_err(); + }; + + join(srv, h2).await; +} + #[tokio::test] async fn early_hints() { h2_support::trace_init!();