mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-11 21:20:39 +02:00
Merge bitcoin/bitcoin#35759: fuzz: check http_request body matches framing
7502b9ddbafuzz: check http_request body matches framing (ameen-alam) Pull request description: The http_request target asserted that ReadBody() returns an empty string. That held for the libevent-based http_libevent::HTTPRequest, where the harness only parsed the request line and headers and never populated a body. Commit9c20859b5f(PR #35182) replaced libevent with http_bitcoin::HTTPRequest, and the target was switched over in e427c227fa; its LoadBody() now decodes Content-Length and chunked bodies per RFC 9112, so any fully-parsed request carrying a body trips the stale assertion (e.g. "POST / HTTP/1.1\r\nContent-Length: 3\r\n\r\nabc"). Replace the emptiness check with a framing-consistency check that mirrors LoadBody()'s own branch logic: a chunked body is bounded by MAX_BODY_SIZE, a Content-Length body is exactly that many bytes, and a request with neither framing header has no body. This strengthens the target instead of dropping the assertion. **Steps to reproduce (old assertion):** Build the fuzz binary and pass this input as a file to the `http_request` target: `POST / HTTP/1.1\r\nContent-Length: 3\r\n\r\nabc` → `test/fuzz/http_request.cpp:49: Assertion 'body.empty()' failed` **Testing the fix:** Ran the updated target ~16 min under libFuzzer with ASAN/UBSAN (14.2M execs, no crashes), plus targeted inputs for each branch: Content-Length body, chunked, `Transfer-Encoding: identity` + Content-Length, no framing headers, and `Content-Length: 0`. Happy to contribute the repro input to qa-assets as a follow-up. ACKs for top commit: pinheadmz: ACK7502b9ddbamarcofleon: tACK7502b9ddbaTree-SHA512: 4f2eb6bdb3a4556866a84fe0f1d0d8cf506e2efd1b1c7493a99f67ca452b31a140034d418c4064142b1a66c3a6c34b97df0e2b12c21ea86cd4019ffc7cff3b27
This commit is contained in:
@@ -45,6 +45,24 @@ FUZZ_TARGET(http_request)
|
||||
(void)http_request.GetHeader(header);
|
||||
(void)http_request.WriteHeader(std::string(header), fuzzed_data_provider.ConsumeRandomLengthString(16));
|
||||
(void)http_request.GetHeader(header);
|
||||
// Reaching here means LoadControlData/LoadHeaders/LoadBody all succeeded, so the
|
||||
// parsed body must be consistent with the message framing. Before libevent was
|
||||
// replaced with http_bitcoin::HTTPRequest (#35182), ReadBody() always returned an
|
||||
// empty string here; LoadBody now populates the body per RFC 9112 framing, so mirror
|
||||
// its branch logic to assert the body matches the framing that produced it.
|
||||
const std::string body = http_request.ReadBody();
|
||||
assert(body.empty());
|
||||
const auto [has_transfer_encoding, transfer_encoding] = http_request.GetHeader("Transfer-Encoding");
|
||||
const auto [has_content_length, content_length] = http_request.GetHeader("Content-Length");
|
||||
if (has_transfer_encoding && ToLower(transfer_encoding) == "chunked") {
|
||||
// A chunked body is the concatenation of the decoded chunks, bounded by MAX_BODY_SIZE.
|
||||
assert(body.size() <= http_bitcoin::MAX_BODY_SIZE);
|
||||
} else if (has_content_length) {
|
||||
// A Content-Length body is exactly that many bytes.
|
||||
const auto parsed_length{ToIntegral<uint64_t>(content_length)};
|
||||
assert(parsed_length);
|
||||
assert(body.size() == *parsed_length);
|
||||
} else {
|
||||
// Absent both framing headers there is no body.
|
||||
assert(body.empty());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user