Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/http/H2Client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment thread
robobun marked this conversation as resolved.
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
Expand Down
3 changes: 3 additions & 0 deletions src/http/h2_client/ClientSession.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion src/http/h2_client/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
Expand Down
99 changes: 98 additions & 1 deletion test/js/web/fetch/fetch-http2-adversarial.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)]);

Expand Down Expand Up @@ -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
Comment thread
robobun marked this conversation as resolved.
]),
);
},
},
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(
Expand Down
Loading