Reject malformed chunk-size in chunked decoder

strtoul silently accepts a leading "-" and wraps via unsigned
arithmetic, so chunk-size "-2" produced ULONG_MAX-1, bypassing the
ULONG_MAX guard and letting a client drive the server toward unbounded
allocation.

Replace strtoul with a manual hex parser that requires at least one hex
digit, detects size_t overflow per digit, and accepts only chunk-ext or
end-of-line after the digits (RFC 9112 §7.1).
This commit is contained in:
yhirose
2026-05-09 16:52:32 +09:00
parent a1fdc07f34
commit 87d62db46b
2 changed files with 50 additions and 5 deletions
+17 -5
View File
@@ -12835,10 +12835,22 @@ inline ssize_t ChunkedDecoder::read_payload(char *buf, size_t len,
stream_line_reader lr(strm, line_buf, sizeof(line_buf));
if (!lr.getline()) { return -1; }
char *endptr = nullptr;
unsigned long chunk_len = std::strtoul(lr.ptr(), &endptr, 16);
if (endptr == lr.ptr()) { return -1; }
if (chunk_len == ULONG_MAX) { return -1; }
// RFC 9112 §7.1: chunk-size = 1*HEXDIG
const char *p = lr.ptr();
int v = 0;
if (!is_hex(*p, v)) { return -1; }
size_t chunk_len = 0;
constexpr size_t chunk_len_max = (std::numeric_limits<size_t>::max)();
for (; is_hex(*p, v); ++p) {
if (chunk_len > (chunk_len_max >> 4)) { return -1; }
chunk_len = (chunk_len << 4) | static_cast<size_t>(v);
}
while (is_space_or_tab(*p)) {
++p;
}
if (*p != '\0' && *p != ';' && *p != '\r' && *p != '\n') { return -1; }
if (chunk_len == 0) {
chunk_remaining = 0;
@@ -12848,7 +12860,7 @@ inline ssize_t ChunkedDecoder::read_payload(char *buf, size_t len,
return 0;
}
chunk_remaining = static_cast<size_t>(chunk_len);
chunk_remaining = chunk_len;
last_chunk_total = chunk_remaining;
last_chunk_offset = 0;
}
+33
View File
@@ -5146,6 +5146,39 @@ TEST_F(ServerTest, CaseInsensitiveTransferEncoding) {
EXPECT_EQ(StatusCode::OK_200, res->status);
}
// GHSA-h6wq-j5mv-f3q8: the server must reject malformed chunk-size lines
// rather than treat them as valid lengths.
template <typename ClientT>
static void expect_chunked_body_rejected(ClientT &cli, const char *body) {
Request req;
req.method = "POST";
req.path = "/chunked";
std::string host_and_port;
host_and_port += HOST;
host_and_port += ":";
host_and_port += std::to_string(PORT);
req.headers.emplace("Host", host_and_port.c_str());
req.headers.emplace("Content-Length", "0");
req.headers.emplace("Transfer-Encoding", "chunked");
req.body = body;
auto res = std::make_shared<Response>();
auto error = Error::Success;
ASSERT_TRUE(cli.send(req, *res, error));
EXPECT_EQ(StatusCode::BadRequest_400, res->status);
}
TEST_F(ServerTest, RejectsNegativeChunkSize) {
expect_chunked_body_rejected(cli_, "-2\r\nAAAA\r\n0\r\n\r\n");
}
TEST_F(ServerTest, RejectsChunkSizeWithLeadingPlus) {
expect_chunked_body_rejected(
cli_, "+4\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n");
}
TEST_F(ServerTest, GetStreamed2) {
auto res = cli_.Get("/streamed", {{make_range_header({{2, 3}})}});
ASSERT_TRUE(res);