mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
Harden chunked encoding parser (#26594)
## Summary - Improve handling of fragmented chunk data in the HTTP parser - Add test coverage for edge cases ## Test plan - [x] New tests pass - [x] Existing tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -204,26 +204,38 @@ namespace uWS {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// do we have data to emit all?
|
// do we have data to emit all?
|
||||||
if (data.length() >= chunkSize(state)) {
|
unsigned int remaining = chunkSize(state);
|
||||||
|
if (data.length() >= remaining) {
|
||||||
// emit all but 2 bytes then reset state to 0 and goto beginning
|
// emit all but 2 bytes then reset state to 0 and goto beginning
|
||||||
// not fin
|
// not fin
|
||||||
std::string_view emitSoon;
|
std::string_view emitSoon;
|
||||||
bool shouldEmit = false;
|
bool shouldEmit = false;
|
||||||
if (chunkSize(state) > 2) {
|
// Validate the chunk terminator (\r\n) accounting for partial reads
|
||||||
emitSoon = std::string_view(data.data(), chunkSize(state) - 2);
|
switch (remaining) {
|
||||||
|
default:
|
||||||
|
// remaining > 2: emit data and validate full terminator
|
||||||
|
emitSoon = std::string_view(data.data(), remaining - 2);
|
||||||
shouldEmit = true;
|
shouldEmit = true;
|
||||||
}
|
[[fallthrough]];
|
||||||
// Validate that the chunk terminator is \r\n to prevent request smuggling
|
case 2:
|
||||||
// The last 2 bytes of the chunk must be exactly \r\n
|
// remaining >= 2: validate both \r and \n
|
||||||
// Note: chunkSize always includes +2 for the terminator (added in consumeHexNumber),
|
if (data[remaining - 2] != '\r' || data[remaining - 1] != '\n') {
|
||||||
// and chunks with size 0 (chunkSize == 2) are handled earlier at line 190.
|
|
||||||
// Therefore chunkSize >= 3 here, so no underflow is possible.
|
|
||||||
size_t terminatorOffset = chunkSize(state) - 2;
|
|
||||||
if (data[terminatorOffset] != '\r' || data[terminatorOffset + 1] != '\n') {
|
|
||||||
state = STATE_IS_ERROR;
|
state = STATE_IS_ERROR;
|
||||||
return std::nullopt;
|
return std::nullopt;
|
||||||
}
|
}
|
||||||
data.remove_prefix(chunkSize(state));
|
break;
|
||||||
|
case 1:
|
||||||
|
// remaining == 1: only \n left to validate
|
||||||
|
if (data[0] != '\n') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
case 0:
|
||||||
|
// remaining == 0: terminator already consumed
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
data.remove_prefix(remaining);
|
||||||
state = STATE_IS_CHUNKED;
|
state = STATE_IS_CHUNKED;
|
||||||
if (shouldEmit) {
|
if (shouldEmit) {
|
||||||
return emitSoon;
|
return emitSoon;
|
||||||
@@ -232,19 +244,45 @@ namespace uWS {
|
|||||||
} else {
|
} else {
|
||||||
/* We will consume all our input data */
|
/* We will consume all our input data */
|
||||||
std::string_view emitSoon;
|
std::string_view emitSoon;
|
||||||
if (chunkSize(state) > 2) {
|
unsigned int size = chunkSize(state);
|
||||||
uint64_t maximalAppEmit = chunkSize(state) - 2;
|
size_t len = data.length();
|
||||||
if (data.length() > maximalAppEmit) {
|
if (size > 2) {
|
||||||
|
uint64_t maximalAppEmit = size - 2;
|
||||||
|
if (len > maximalAppEmit) {
|
||||||
emitSoon = data.substr(0, maximalAppEmit);
|
emitSoon = data.substr(0, maximalAppEmit);
|
||||||
|
// Validate terminator bytes being consumed
|
||||||
|
size_t terminatorBytesConsumed = len - maximalAppEmit;
|
||||||
|
if (terminatorBytesConsumed >= 1 && data[maximalAppEmit] != '\r') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
|
}
|
||||||
|
if (terminatorBytesConsumed >= 2 && data[maximalAppEmit + 1] != '\n') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
//cb(data);
|
|
||||||
emitSoon = data;
|
emitSoon = data;
|
||||||
}
|
}
|
||||||
|
} else if (size == 2) {
|
||||||
|
// Only terminator bytes remain, validate what we have
|
||||||
|
if (len >= 1 && data[0] != '\r') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
}
|
}
|
||||||
decChunkSize(state, (unsigned int) data.length());
|
if (len >= 2 && data[1] != '\n') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
|
}
|
||||||
|
} else if (size == 1) {
|
||||||
|
// Only \n remains
|
||||||
|
if (data[0] != '\n') {
|
||||||
|
state = STATE_IS_ERROR;
|
||||||
|
return std::nullopt;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
decChunkSize(state, (unsigned int) len);
|
||||||
state |= STATE_IS_CHUNKED;
|
state |= STATE_IS_CHUNKED;
|
||||||
// new: decrease data by its size (bug)
|
data.remove_prefix(len);
|
||||||
data.remove_prefix(data.length()); // ny bug fix för getNextChunk
|
|
||||||
if (emitSoon.length()) {
|
if (emitSoon.length()) {
|
||||||
return emitSoon;
|
return emitSoon;
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -1,7 +1,112 @@
|
|||||||
import type { Socket } from "bun";
|
import type { Socket } from "bun";
|
||||||
import { setSocketOptions } from "bun:internal-for-testing";
|
import { setSocketOptions } from "bun:internal-for-testing";
|
||||||
import { describe, test } from "bun:test";
|
import { describe, expect, test } from "bun:test";
|
||||||
import { isPosix } from "harness";
|
import { bunEnv, bunExe, isPosix } from "harness";
|
||||||
|
|
||||||
|
describe.if(isPosix)("HTTP server handles chunked transfer encoding", () => {
|
||||||
|
test("handles fragmented chunk terminators", async () => {
|
||||||
|
const script = `
|
||||||
|
const server = Bun.serve({
|
||||||
|
port: 0,
|
||||||
|
async fetch(req) {
|
||||||
|
const body = await req.text();
|
||||||
|
return new Response("Got: " + body);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const { promise, resolve } = Promise.withResolvers();
|
||||||
|
const socket = await Bun.connect({
|
||||||
|
hostname: "localhost",
|
||||||
|
port: server.port,
|
||||||
|
socket: {
|
||||||
|
data(socket, data) {
|
||||||
|
console.log(data.toString());
|
||||||
|
socket.end();
|
||||||
|
},
|
||||||
|
open(socket) {
|
||||||
|
socket.write("POST / HTTP/1.1\\r\\nHost: localhost\\r\\nTransfer-Encoding: chunked\\r\\n\\r\\n4\\r\\nWiki\\r");
|
||||||
|
socket.flush();
|
||||||
|
setTimeout(() => {
|
||||||
|
socket.write("\\n0\\r\\n\\r\\n");
|
||||||
|
socket.flush();
|
||||||
|
}, 50);
|
||||||
|
},
|
||||||
|
error() {},
|
||||||
|
close() { resolve(); },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
await promise;
|
||||||
|
server.stop();
|
||||||
|
`;
|
||||||
|
|
||||||
|
await using proc = Bun.spawn({
|
||||||
|
cmd: [bunExe(), "-e", script],
|
||||||
|
env: bunEnv,
|
||||||
|
stdout: "pipe",
|
||||||
|
stderr: "pipe",
|
||||||
|
});
|
||||||
|
|
||||||
|
const [stdout, stderr, exitCode] = await Promise.all([
|
||||||
|
new Response(proc.stdout).text(),
|
||||||
|
new Response(proc.stderr).text(),
|
||||||
|
proc.exited,
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(stdout).toContain("200 OK");
|
||||||
|
expect(stdout).toContain("Got: Wiki");
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("rejects invalid terminator in fragmented reads", async () => {
|
||||||
|
const script = `
|
||||||
|
const server = Bun.serve({
|
||||||
|
port: 0,
|
||||||
|
async fetch(req) {
|
||||||
|
const body = await req.text();
|
||||||
|
return new Response("Got: " + body);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const { promise, resolve } = Promise.withResolvers();
|
||||||
|
const socket = await Bun.connect({
|
||||||
|
hostname: "localhost",
|
||||||
|
port: server.port,
|
||||||
|
socket: {
|
||||||
|
data(socket, data) {
|
||||||
|
console.log(data.toString());
|
||||||
|
socket.end();
|
||||||
|
},
|
||||||
|
open(socket) {
|
||||||
|
socket.write("POST / HTTP/1.1\\r\\nHost: localhost\\r\\nTransfer-Encoding: chunked\\r\\n\\r\\n4\\r\\nTestX");
|
||||||
|
socket.flush();
|
||||||
|
setTimeout(() => {
|
||||||
|
socket.write("\\n0\\r\\n\\r\\n");
|
||||||
|
socket.flush();
|
||||||
|
}, 50);
|
||||||
|
},
|
||||||
|
error() {},
|
||||||
|
close() { resolve(); },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
await promise;
|
||||||
|
server.stop();
|
||||||
|
`;
|
||||||
|
|
||||||
|
await using proc = Bun.spawn({
|
||||||
|
cmd: [bunExe(), "-e", script],
|
||||||
|
env: bunEnv,
|
||||||
|
stdout: "pipe",
|
||||||
|
stderr: "pipe",
|
||||||
|
});
|
||||||
|
|
||||||
|
const [stdout, stderr, exitCode] = await Promise.all([
|
||||||
|
new Response(proc.stdout).text(),
|
||||||
|
new Response(proc.stderr).text(),
|
||||||
|
proc.exited,
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(stdout).toContain("400");
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe.if(isPosix)("HTTP server handles fragmented requests", () => {
|
describe.if(isPosix)("HTTP server handles fragmented requests", () => {
|
||||||
test("handles requests with tiny send buffer (regression test)", async () => {
|
test("handles requests with tiny send buffer (regression test)", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user