diff --git a/httplib.h b/httplib.h index 97d48c80..4f221f84 100644 --- a/httplib.h +++ b/httplib.h @@ -15301,22 +15301,45 @@ 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; } + // Everything below is bounded by eol rather than by the buffer's NUL, so + // the line terminator is never mistaken for line content. + const char *eol = lr.ptr() + lr.size(); + if (lr.end_with_crlf()) { + eol -= 2; + } else if (eol != lr.ptr() && eol[-1] == '\n') { + // Only reachable under CPPHTTPLIB_ALLOW_LF_AS_LINE_TERMINATOR, where + // getline() ends the line on a bare LF. That LF is the terminator, so it + // has to come off here or the check below would reject the line. + eol -= 1; + } + // RFC 9112 §7.1: chunk-size = 1*HEXDIG const char *p = lr.ptr(); int v = 0; - if (!is_hex(*p, v)) { return -1; } + if (p == eol || !is_hex(*p, v)) { return -1; } size_t chunk_len = 0; constexpr size_t chunk_len_max = (std::numeric_limits::max)(); - for (; is_hex(*p, v); ++p) { + for (; p < eol && is_hex(*p, v); ++p) { if (chunk_len > (chunk_len_max >> 4)) { return -1; } chunk_len = (chunk_len << 4) | static_cast(v); } - while (is_space_or_tab(*p)) { + while (p < eol && is_space_or_tab(*p)) { ++p; } - if (*p != '\0' && *p != ';' && *p != '\r' && *p != '\n') { return -1; } + + // RFC 9112 §7.1.1: only a chunk-ext may sit between the size and the line + // terminator, and it is built from tokens and quoted-strings, so it never + // holds a CR, LF or any other control character. getline() reads up to the + // CRLF, so a bare LF left in here would be swallowed as extension text + // while an intermediary that ends the line on it delimits the chunks + // differently, and the two disagree on where the body ends (request + // smuggling). + if (p < eol && *p != ';') { return -1; } + for (; p < eol; ++p) { + if (!is_space_or_tab(*p) && !fields::is_field_vchar(*p)) { return -1; } + } if (chunk_len == 0) { chunk_remaining = 0; diff --git a/test/test.cc b/test/test.cc index 48834a49..f8840cfd 100644 --- a/test/test.cc +++ b/test/test.cc @@ -6987,10 +6987,9 @@ 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 -static void expect_chunked_body_rejected(ClientT &cli, const char *body) { +static void expect_chunked_body_status(ClientT &cli, const char *body, + int expected_status) { Request req; req.method = "POST"; req.path = "/chunked"; @@ -7008,7 +7007,14 @@ static void expect_chunked_body_rejected(ClientT &cli, const char *body) { auto res = std::make_shared(); auto error = Error::Success; ASSERT_TRUE(cli.send(req, *res, error)); - EXPECT_EQ(StatusCode::BadRequest_400, res->status); + EXPECT_EQ(expected_status, res->status); +} + +// GHSA-h6wq-j5mv-f3q8: the server must reject malformed chunk-size lines +// rather than treat them as valid lengths. +template +static void expect_chunked_body_rejected(ClientT &cli, const char *body) { + expect_chunked_body_status(cli, body, StatusCode::BadRequest_400); } TEST_F(ServerTest, RejectsNegativeChunkSize) { @@ -7020,6 +7026,40 @@ TEST_F(ServerTest, RejectsChunkSizeWithLeadingPlus) { cli_, "+4\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n"); } +// RFC 9112 §7.1.1: a chunk-ext is made of tokens and quoted-strings, so the +// chunk-size line carries no CR, LF or other control character ahead of its +// terminator. Such a line must be refused rather than read as extension text. +TEST_F(ServerTest, RejectsBareLFInChunkExtension) { + expect_chunked_body_rejected( + cli_, "4;\nxx\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n"); +} + +TEST_F(ServerTest, RejectsBareLFAfterChunkSize) { + expect_chunked_body_rejected( + cli_, "4\nxx\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n"); +} + +TEST_F(ServerTest, RejectsBareCRInChunkExtension) { + expect_chunked_body_rejected( + cli_, "4;a\rb\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n"); +} + +TEST_F(ServerTest, RejectsControlCharacterInChunkExtension) { + // The literal stays split: a hex escape consumes every hex digit that + // follows, so "\x01b" would be the single byte \x1b, not \x01 then 'b'. + expect_chunked_body_rejected( + cli_, "4;a\x01" + "b\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n"); +} + +TEST_F(ServerTest, AcceptsChunkExtension) { + expect_chunked_body_status(cli_, + "4;name=value\r\ndech\r\n" + "f ; note=\"a;b c\"\r\nunked post body\r\n" + "0;last\r\n\r\n", + StatusCode::OK_200); +} + TEST_F(ServerTest, GetStreamed2) { auto res = cli_.Get("/streamed", Headers{{make_range_header({{2, 3}})}}); ASSERT_TRUE(res) << "Error: " << to_string(res.error());