From 6e5ee24ad92687a56a26ae596da45dc6c2f902c0 Mon Sep 17 00:00:00 2001 From: Kenton Varda Date: Tue, 10 Mar 2026 18:16:14 -0500 Subject: [PATCH] Fix HTTP body size integer overflow bugs. The KJ-HTTP library was discovered to have two bugs related to integer overflows while handling message body sizes: 1. A negative `Content-Length` value was converted to unsigned, treating it as an impossibly large length instead. 2. When using `Transfer-Encoding: chunked`, if a chunk's size parsed to a value of 2^64 or larger, it would be truncated to a 64-bit integer. In theory, these bugs could enable HTTP request/response smuggling, although it would require integration with a proxy that has bugs of its own. For more details, see (in a future commit): security-advisories/2026-03-10-1-http-size-validation.md CVE: CVE-2026-32239 CVE: CVE-2026-32240 Upstream: https://github.com/capnproto/capnproto/commit/2744b3c012b4aa3c31cefb61ec656829fa5c0e36 Signed-off-by: Thomas Perale --- c++/src/kj/compat/http-test.c++ | 64 +++++++++++++++++++++++++++++++++ c++/src/kj/compat/http.c++ | 28 +++++++++++---- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/c++/src/kj/compat/http-test.c++ b/c++/src/kj/compat/http-test.c++ index f10ff8d156..daf08992f0 100644 --- a/c++/src/kj/compat/http-test.c++ +++ b/c++/src/kj/compat/http-test.c++ @@ -2659,6 +2659,70 @@ KJ_TEST("HttpServer invalid method") { KJ_EXPECT(expectedResponse == response, expectedResponse, response); } +KJ_TEST("HttpServer rejects negative Content-Length") { + KJ_HTTP_TEST_SETUP_IO; + kj::TimerImpl timer(kj::origin()); + auto pipe = KJ_HTTP_TEST_CREATE_2PIPE; + + HttpHeaderTable table; + BrokenHttpService service; + HttpServer server(timer, table, service, { + .canceledUploadGraceBytes = 1024 * 1024, + }); + + auto listenTask = server.listenHttp(kj::mv(pipe.ends[0])); + + auto msg = + "POST / HTTP/1.1\r\n" + "Content-Length: -1\r\n" + "\r\n" + "foo"_kj.asBytes(); + + auto writePromise = pipe.ends[1]->write(msg.begin(), msg.size()); + auto response = pipe.ends[1]->readAllText().wait(waitScope); + + // The server should reject the negative Content-Length. The KJ_FAIL_REQUIRE in getEntityBody() + // gets caught by the server loop and turned into a 500 error. + KJ_EXPECT(response.startsWith("HTTP/1.1 500 Internal Server Error"), response); + + KJ_EXPECT(writePromise.poll(waitScope)); + writePromise.catch_([](kj::Exception&&) {}).wait(waitScope); +} + +KJ_TEST("HttpServer rejects chunked body with overflowing chunk size") { + KJ_HTTP_TEST_SETUP_IO; + kj::TimerImpl timer(kj::origin()); + auto pipe = KJ_HTTP_TEST_CREATE_2PIPE; + + HttpHeaderTable table; + BrokenHttpService service; + HttpServer server(timer, table, service, { + .canceledUploadGraceBytes = 1024 * 1024, + }); + + auto listenTask = server.listenHttp(kj::mv(pipe.ends[0])); + + // 17 hex digits: 0x10000000000000000 = 2^64, which overflows uint64_t. + auto msg = + "POST / HTTP/1.1\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "10000000000000000\r\n" + "x\r\n" + "0\r\n" + "\r\n"_kj.asBytes(); + + auto writePromise = pipe.ends[1]->write(msg.begin(), msg.size()); + auto response = pipe.ends[1]->readAllText().wait(waitScope); + + // The chunk size overflow causes a KJ_REQUIRE failure during body reading, which the server + // catches and turns into a 500 error. + KJ_EXPECT(response.startsWith("HTTP/1.1 500 Internal Server Error"), response); + + KJ_EXPECT(writePromise.poll(waitScope)); + writePromise.catch_([](kj::Exception&&) {}).wait(waitScope); +} + // Ensure that HttpServerSettings can continue to be constexpr. KJ_UNUSED static constexpr HttpServerSettings STATIC_CONSTEXPR_SETTINGS {}; diff --git a/c++/src/kj/compat/http.c++ b/c++/src/kj/compat/http.c++ index 6976335e9c..6dc1e48613 100644 --- a/c++/src/kj/compat/http.c++ +++ b/c++/src/kj/compat/http.c++ @@ -1246,16 +1246,20 @@ public: uint64_t value = 0; for (char c: text) { + uint64_t digit; if ('0' <= c && c <= '9') { - value = value * 16 + (c - '0'); + digit = c - '0'; } else if ('a' <= c && c <= 'f') { - value = value * 16 + (c - 'a' + 10); + digit = c - 'a' + 10; } else if ('A' <= c && c <= 'F') { - value = value * 16 + (c - 'A' + 10); + digit = c - 'A' + 10; } else { KJ_FAIL_REQUIRE("invalid HTTP chunk size", text, text.asBytes()) { break; } return value; } + KJ_REQUIRE(value <= (uint64_t(kj::maxValue) >> 4), + "HTTP chunk size overflow", text, text.asBytes()) { break; } + value = value * 16 + digit; } return value; @@ -1742,7 +1746,15 @@ kj::Own HttpInputStreamImpl::getEntityBody( // Body elided. kj::Maybe length; KJ_IF_MAYBE(cl, headers.get(HttpHeaderId::CONTENT_LENGTH)) { - length = strtoull(cl->cStr(), nullptr, 10); + // Validate that the Content-Length is a non-negative integer. Note that strtoull() accepts + // leading '-' signs and silently converts negative values to large unsigned values, so we + // must explicitly check for a leading digit. + char* end; + uint64_t parsedValue = strtoull(cl->cStr(), &end, 10); + if ((*cl)[0] >= '0' && (*cl)[0] <= '9' && end > cl->begin() && *end == '\0') { + length = parsedValue; + } + // If invalid, we just leave `length` as nullptr, since the body is elided anyway. } else if (headers.get(HttpHeaderId::TRANSFER_ENCODING) == nullptr) { // HACK: Neither Content-Length nor Transfer-Encoding header in response to HEAD request. // Propagate this fact with a 0 expected body length. @@ -1786,12 +1798,16 @@ kj::Own HttpInputStreamImpl::getEntityBody( // "Content-Length: 5, 5, 5". Hopefully no one actually does that... char* end; uint64_t length = strtoull(cl->cStr(), &end, 10); - if (end > cl->begin() && *end == '\0') { + // Note that strtoull() accepts leading '-' signs and silently converts negative values to + // large unsigned values, so we must explicitly check for a leading digit. + if ((*cl)[0] >= '0' && (*cl)[0] <= '9' && end > cl->begin() && *end == '\0') { // #5 return kj::heap(*this, length); } else { // #4 (bad content-length) - KJ_FAIL_REQUIRE("invalid Content-Length header value", *cl); + KJ_FAIL_REQUIRE("invalid Content-Length header value", *cl) { break; } + // To pass the -fno-exceptions test (but KJ-HTTP is really not safe to use in that mode). + return kj::heap(*this, uint64_t(0)); } }