diff --git a/src/http/H2Client.rs b/src/http/H2Client.rs index 1a3e355361e5..a946e78c8fd4 100644 --- a/src/http/H2Client.rs +++ b/src/http/H2Client.rs @@ -22,6 +22,10 @@ pub(crate) const LOCAL_INITIAL_WINDOW_SIZE: u32 = 1 << 24; /// cap is checked locally regardless of what the server honors. pub(crate) const LOCAL_MAX_HEADER_LIST_SIZE: u32 = 256 * 1024; +/// CONTINUATION frames allowed per header block. Matches nghttp2's +/// `NGHTTP2_DEFAULT_MAX_CONTINUATIONS` (CVE-2024-28182). +pub(crate) const LOCAL_MAX_CONTINUATIONS: u8 = 8; + /// `write_buffer` high-water mark. `writeDataWindowed` stops queueing once the /// userland send buffer crosses this even if flow-control window remains, so a /// large grant doesn't duplicate the whole body in memory before the first diff --git a/src/http/h2_client/ClientSession.rs b/src/http/h2_client/ClientSession.rs index b2639af4e5f7..580af0a9510c 100644 --- a/src/http/h2_client/ClientSession.rs +++ b/src/http/h2_client/ClientSession.rs @@ -81,6 +81,8 @@ pub struct ClientSession { pub(crate) next_stream_id: u31, /// Stream id whose CONTINUATION sequence is in progress; 0 = none. pub(crate) expecting_continuation: u31, + /// CONTINUATION frames seen so far in the current header block. + pub(crate) continuation_count: u8, /// Cold-start coalesced requests parked until the server's first SETTINGS /// frame arrives so the real MAX_CONCURRENT_STREAMS cap can be honoured. @@ -350,6 +352,7 @@ impl ClientSession { by_http_id: ArrayHashMap::default(), next_stream_id: 1, expecting_continuation: 0, + continuation_count: 0, pending_attach: Vec::new(), preface_sent: false, settings_received: false, diff --git a/src/http/h2_client/dispatch.rs b/src/http/h2_client/dispatch.rs index 82e7bbaa04fa..40ec9a6f32c9 100644 --- a/src/http/h2_client/dispatch.rs +++ b/src/http/h2_client/dispatch.rs @@ -5,7 +5,7 @@ use super::client_session::{ClientSession, stream_mut}; use super::stream::{State as StreamState, Stream}; -use super::{LOCAL_MAX_HEADER_LIST_SIZE, WRITE_BUFFER_CONTROL_LIMIT}; +use super::{LOCAL_MAX_CONTINUATIONS, LOCAL_MAX_HEADER_LIST_SIZE, WRITE_BUFFER_CONTROL_LIMIT}; use crate::h2_frame_parser as wire; use bun_picohttp as picohttp; @@ -293,6 +293,7 @@ fn dispatch_frame( } } FT_HEADERS => { + session.continuation_count = 0; let mut fragment = payload; let maybe_stream = session.streams.get(&stream_id).copied(); if maybe_stream.is_none() { @@ -380,6 +381,11 @@ fn dispatch_frame( session.fatal_error = Some(crate::Error::HTTP2ProtocolError); return; } + session.continuation_count += 1; + if session.continuation_count > LOCAL_MAX_CONTINUATIONS { + session.fatal_error = Some(crate::Error::HTTP2EnhanceYourCalm); + return; + } if let Some(&stream_ptr) = session.streams.get(&session.expecting_continuation) { // SAFETY: stream pointer valid for session lifetime. let stream = stream_mut(stream_ptr); diff --git a/test/js/web/fetch/fetch-http2-adversarial.test.ts b/test/js/web/fetch/fetch-http2-adversarial.test.ts index 15d34d3ddf69..f5bdd98f3b1f 100644 --- a/test/js/web/fetch/fetch-http2-adversarial.test.ts +++ b/test/js/web/fetch/fetch-http2-adversarial.test.ts @@ -26,6 +26,8 @@ const setting = (id: number, value: number) => { return b; }; const hpackStatus200 = Buffer.from([0x80 | 8]); +// Literal field, indexed name (:status is static index 8), literal value. +const hpackStatus = (code: string) => Buffer.concat([Buffer.from([0x08, code.length]), Buffer.from(code)]); const hpackLit = (name: string, value: string) => Buffer.concat([Buffer.from([0x10, name.length]), Buffer.from(name), Buffer.from([value.length]), Buffer.from(value)]); @@ -159,13 +161,108 @@ describe.concurrent("fetch() HTTP/2 adversarial", () => { // Connection should error well before the ~80 MB of CONTINUATION payload // accumulates. Allow generous slack for TLS/allocator overhead. // ASAN's quarantine retains freed allocations so widen the threshold there. - expect(out.result).toBe("HTTP2HeaderListTooLarge"); + // The frame-count cap (8 frames x ~16 KB = ~128 KB) fires before the + // 256 KiB byte-size cap for this payload shape. + expect(out.result).toBe("HTTP2EnhanceYourCalm"); expect(out.growth).toBeLessThan((isASAN ? 256 : 64) * 1024 * 1024); expect(exitCode).toBe(0); }, ); }); + // 1b. Zero-length CONTINUATION flood (CVE-2024-28182): empty payloads never + // advance the byte-size cap above, so bound the frame count too. + test("zero-length CONTINUATION flood is rejected", async () => { + await withAdversarialServer( + { + onStream: (socket, id) => { + // HEADERS :status 200, no END_HEADERS, no END_STREAM. + socket.write(frame(1, 0, id, hpackStatus200)); + // 10 000 empty CONTINUATION frames = 90 000 bytes of 9-byte headers + // with zero payload; header_block.len() stays at 1 forever. + const flood = Buffer.concat(Array.from({ length: 10_000 }, () => frame(9, 0, id))); + socket.write(flood); + }, + }, + async url => { + const code = await fetch(url, { ...h2, signal: AbortSignal.timeout(8000) }).then(r => r.status, errcode); + expect(code).toBe("HTTP2EnhanceYourCalm"); + }, + ); + }); + + // 1c. Boundary: exactly the CONTINUATION budget (8 frames) is accepted. + test("header block split across the full CONTINUATION budget is accepted", async () => { + await withAdversarialServer( + { + onStream: (socket, id) => { + socket.write( + Buffer.concat([ + frame(1, 0x1, id, hpackStatus200), // HEADERS, END_STREAM, no END_HEADERS + ...Array.from({ length: 7 }, () => frame(9, 0, id)), + frame(9, 0x4, id), // 8th CONTINUATION, END_HEADERS + ]), + ); + }, + }, + async url => { + const r = await fetch(url, h2); + expect(r.status).toBe(200); + expect(await r.text()).toBe(""); + }, + ); + }); + + // 1d. Boundary: the 9th CONTINUATION is rejected even when it carries + // END_HEADERS and would have completed the block (nghttp2 / node agree). + test("9th CONTINUATION is rejected even if it ends the block", async () => { + await withAdversarialServer( + { + onStream: (socket, id) => { + socket.write( + Buffer.concat([ + frame(1, 0x1, id, hpackStatus200), // HEADERS, END_STREAM, no END_HEADERS + ...Array.from({ length: 8 }, () => frame(9, 0, id)), + frame(9, 0x4, id), // 9th CONTINUATION, END_HEADERS + ]), + ); + }, + }, + async url => { + const code = await fetch(url, h2).then(r => r.status, errcode); + expect(code).toBe("HTTP2EnhanceYourCalm"); + }, + ); + }); + + // 1e. The budget is per header block, not per stream or per connection: a + // 103 block and the final 200 block on the same stream may each use all + // 8 frames (16 CONTINUATIONs total on one stream). + test("CONTINUATION budget resets for each header block on a stream", async () => { + await withAdversarialServer( + { + onStream: (socket, id) => { + const splitBlock = (headersFlags: number, block: Buffer) => [ + frame(1, headersFlags, id, block), // no END_HEADERS + ...Array.from({ length: 7 }, () => frame(9, 0, id)), + frame(9, 0x4, id), // 8th CONTINUATION, END_HEADERS + ]; + socket.write( + Buffer.concat([ + ...splitBlock(0, hpackStatus("103")), // informational, stream stays open + ...splitBlock(0x1, hpackStatus200), // final response, END_STREAM + ]), + ); + }, + }, + async url => { + const r = await fetch(url, h2); + expect(r.status).toBe(200); + expect(await r.text()).toBe(""); + }, + ); + }); + // 2. MAX_CONCURRENT_STREAMS=0: server allows zero streams. test("MAX_CONCURRENT_STREAMS=0 does not hang", async () => { await withAdversarialServer(