From 6fd97aeca0faa1c6e1bd7ae8150c821dcff31c3b Mon Sep 17 00:00:00 2001 From: yhirose Date: Fri, 27 Mar 2026 23:16:08 -0400 Subject: [PATCH] Implement request body consumption and reject invalid Content-Length with Transfer-Encoding to prevent request smuggling --- httplib.h | 72 +++++++++++++++++++------- test/test.cc | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 19 deletions(-) diff --git a/httplib.h b/httplib.h index 06c1fdf0..cf688c88 100644 --- a/httplib.h +++ b/httplib.h @@ -1270,6 +1270,7 @@ struct Request { bool is_multipart_form_data() const; // private members... + bool body_consumed_ = false; size_t redirect_count_ = CPPHTTPLIB_REDIRECT_MAX_COUNT; size_t content_length_ = 0; ContentProvider content_provider_; @@ -11288,6 +11289,8 @@ inline bool Server::read_content_core( return false; } + req.body_consumed_ = true; + if (req.is_multipart_form_data()) { if (!multipart_form_data_parser.is_valid()) { res.status = StatusCode::BadRequest_400; @@ -11906,8 +11909,19 @@ Server::process_request(Stream &strm, const std::string &remote_addr, return write_response(strm, close_connection, req, res); } + // RFC 9112 §6.3: Reject requests with both a non-zero Content-Length and + // any Transfer-Encoding to prevent request smuggling. Content-Length: 0 is + // tolerated for compatibility with existing clients. + if (req.get_header_value_u64("Content-Length") > 0 && + req.has_header("Transfer-Encoding")) { + connection_closed = true; + res.status = StatusCode::BadRequest_400; + return write_response(strm, close_connection, req, res); + } + // Check if the request URI doesn't exceed the limit if (req.target.size() > CPPHTTPLIB_REQUEST_URI_MAX_LENGTH) { + connection_closed = true; res.status = StatusCode::UriTooLong_414; output_error_log(Error::ExceedUriMaxLength, &req); return write_response(strm, close_connection, req, res); @@ -11936,6 +11950,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr, if (req.has_header("Accept")) { const auto &accept_header = req.get_header_value("Accept"); if (!detail::parse_accept_header(accept_header, req.accept_content_types)) { + connection_closed = true; res.status = StatusCode::BadRequest_400; output_error_log(Error::HTTPParsing, &req); return write_response(strm, close_connection, req, res); @@ -11945,6 +11960,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr, if (req.has_header("Range")) { const auto &range_header_value = req.get_header_value("Range"); if (!detail::parse_range_header(range_header_value, req.ranges)) { + connection_closed = true; res.status = StatusCode::RangeNotSatisfiable_416; output_error_log(Error::InvalidRangeHeader, &req); return write_response(strm, close_connection, req, res); @@ -12072,6 +12088,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr, } } #endif + auto ret = false; if (routed) { if (res.status == -1) { res.status = req.ranges.empty() ? StatusCode::OK_200 @@ -12079,6 +12096,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr, } // Serve file content by using a content provider + auto file_open_error = false; if (!res.file_content_path_.empty()) { const auto &path = res.file_content_path_; auto mm = std::make_shared(path.c_str()); @@ -12088,37 +12106,53 @@ Server::process_request(Stream &strm, const std::string &remote_addr, res.content_provider_ = nullptr; res.status = StatusCode::NotFound_404; output_error_log(Error::OpenFile, &req); - return write_response(strm, close_connection, req, res); - } + file_open_error = true; + } else { + auto content_type = res.file_content_content_type_; + if (content_type.empty()) { + content_type = detail::find_content_type( + path, file_extension_and_mimetype_map_, default_file_mimetype_); + } - auto content_type = res.file_content_content_type_; - if (content_type.empty()) { - content_type = detail::find_content_type( - path, file_extension_and_mimetype_map_, default_file_mimetype_); + res.set_content_provider( + mm->size(), content_type, + [mm](size_t offset, size_t length, DataSink &sink) -> bool { + sink.write(mm->data() + offset, length); + return true; + }); } - - res.set_content_provider( - mm->size(), content_type, - [mm](size_t offset, size_t length, DataSink &sink) -> bool { - sink.write(mm->data() + offset, length); - return true; - }); } - if (detail::range_error(req, res)) { + if (file_open_error) { + ret = write_response(strm, close_connection, req, res); + } else if (detail::range_error(req, res)) { res.body.clear(); res.content_length_ = 0; res.content_provider_ = nullptr; res.status = StatusCode::RangeNotSatisfiable_416; - return write_response(strm, close_connection, req, res); + ret = write_response(strm, close_connection, req, res); + } else { + ret = write_response_with_content(strm, close_connection, req, res); } - - return write_response_with_content(strm, close_connection, req, res); } else { if (res.status == -1) { res.status = StatusCode::NotFound_404; } - - return write_response(strm, close_connection, req, res); + ret = write_response(strm, close_connection, req, res); } + + // Drain any unconsumed request body to prevent request smuggling on + // keep-alive connections. + if (!req.body_consumed_ && detail::expect_content(req)) { + int drain_status = 200; // required by read_content signature + if (!detail::read_content( + strm, req, payload_max_length_, drain_status, nullptr, + [](const char *, size_t, size_t, size_t) { return true; }, false)) { + // Body exceeds payload limit or read error — close the connection + // to prevent leftover bytes from being misinterpreted. + connection_closed = true; + } + } + + return ret; } inline bool Server::is_valid() const { return true; } diff --git a/test/test.cc b/test/test.cc index fd719462..f5a10397 100644 --- a/test/test.cc +++ b/test/test.cc @@ -17266,3 +17266,146 @@ TEST(SymlinkTest, SymlinkEscapeFromBaseDirectory) { EXPECT_EQ(StatusCode::Forbidden_403, res->status); } #endif + +TEST(RequestSmugglingTest, UnconsumedGETBodyOnFileHandler) { + // A GET request with Content-Length to a static file handler must have its + // body drained before the keep-alive connection is reused. Otherwise the + // unread body bytes are interpreted as the next HTTP request. + // + // The body is sent AFTER receiving the first response (as in the original + // PoC) so that the stream_line_reader cannot buffer it together with the + // headers of the first request. + Server svr; + svr.set_mount_point("/", "./www"); + + std::atomic smuggled_count(0); + svr.Get("/smuggled", [&](const Request &, Response &res) { + smuggled_count++; + res.set_content("oops", "text/plain"); + }); + + auto port = svr.bind_to_any_port("localhost"); + thread t = thread([&] { svr.listen_after_bind(); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + }); + svr.wait_until_ready(); + + auto error = Error::Success; + auto sock = detail::create_client_socket( + "localhost", "", port, AF_UNSPEC, false, false, nullptr, + /*connection_timeout_sec=*/2, 0, + /*read_timeout_sec=*/2, 0, + /*write_timeout_sec=*/2, 0, std::string(), error); + ASSERT_NE(INVALID_SOCKET, sock); + auto sock_se = detail::scope_exit([&] { detail::close_socket(sock); }); + + // The "smuggled" request will be sent as the body of the outer GET + std::string smuggled = "GET /smuggled HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + "\r\n"; + + // Step 1: Send only the outer request headers (no body yet) + std::string outer_headers = "GET /file HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: " + + std::to_string(smuggled.size()) + + "\r\n" + "\r\n"; + + auto sent = + send(sock, outer_headers.data(), outer_headers.size(), MSG_NOSIGNAL); + ASSERT_EQ(static_cast(outer_headers.size()), sent); + + // Step 2: Read the first response (server serves file without reading body) + std::string first_response; + char buf[4096]; + for (;;) { + auto n = recv(sock, buf, sizeof(buf), 0); + if (n <= 0) break; + first_response.append(buf, static_cast(n)); + // Stop once we have a complete response (headers + body) + auto hdr_end = first_response.find("\r\n\r\n"); + if (hdr_end != std::string::npos) { + // Check for Content-Length to know when the body is complete + auto cl_pos = first_response.find("Content-Length:"); + if (cl_pos != std::string::npos) { + auto cl_val_start = cl_pos + 15; // length of "Content-Length:" + auto cl_val_end = first_response.find("\r\n", cl_val_start); + auto cl = std::stoul( + first_response.substr(cl_val_start, cl_val_end - cl_val_start)); + if (first_response.size() >= hdr_end + 4 + cl) { break; } + } else { + break; // No Content-Length, assume headers-only response + } + } + } + ASSERT_TRUE(first_response.find("HTTP/1.1 200") != std::string::npos); + + // Step 3: Now send the body, which looks like a new HTTP request. + // On a vulnerable server the keep-alive loop reads this as a second request. + sent = send(sock, smuggled.data(), smuggled.size(), MSG_NOSIGNAL); + ASSERT_EQ(static_cast(smuggled.size()), sent); + + // Step 4: Try to read a second response (should NOT exist after fix) + std::string second_response; + for (;;) { + auto n = recv(sock, buf, sizeof(buf), 0); + if (n <= 0) break; + second_response.append(buf, static_cast(n)); + } + + // The smuggled request must NOT have been processed + EXPECT_EQ(0, smuggled_count.load()); +} + +TEST(RequestSmugglingTest, ContentLengthAndTransferEncodingRejected) { + // RFC 9112 §6.3: A request with both Content-Length and Transfer-Encoding + // must be rejected with 400 Bad Request. + Server svr; + svr.Post("/test", [&](const Request &, Response &res) { + res.set_content("ok", "text/plain"); + }); + + thread t = thread([&] { svr.listen(HOST, PORT); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + svr.wait_until_ready(); + + // Exact "chunked" + { + auto req = "POST /test HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 5\r\n" + "Transfer-Encoding: chunked\r\n" + "Connection: close\r\n" + "\r\n" + "hello"; + + std::string response; + ASSERT_TRUE(send_request(1, req, &response)); + EXPECT_EQ("HTTP/1.1 400 Bad Request", + response.substr(0, response.find("\r\n"))); + } + + // Multi-valued Transfer-Encoding (e.g., "gzip, chunked") + { + auto req = "POST /test HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 5\r\n" + "Transfer-Encoding: gzip, chunked\r\n" + "Connection: close\r\n" + "\r\n" + "hello"; + + std::string response; + ASSERT_TRUE(send_request(1, req, &response)); + EXPECT_EQ("HTTP/1.1 400 Bad Request", + response.substr(0, response.find("\r\n"))); + } +}