From 8d2f537201ab367994ba8feb3526f37199665865 Mon Sep 17 00:00:00 2001 From: Sayed Kaif Date: Mon, 13 Jul 2026 21:11:00 +0530 Subject: [PATCH 1/3] reject crlf in request target in write_request_line --- httplib.h | 7 +++++++ test/test.cc | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/httplib.h b/httplib.h index 7f9813e7f9..531260f570 100644 --- a/httplib.h +++ b/httplib.h @@ -7504,6 +7504,13 @@ bool read_content(Stream &strm, T &x, size_t payload_max_length, int &status, inline ssize_t write_request_line(Stream &strm, const std::string &method, const std::string &path) { + // A request target must not carry CR/LF (or other control octets); otherwise + // a value smuggled into it splits the request line and injects headers or a + // whole request. The same field-value check already guards header values in + // check_and_write_headers and the request target in + // perform_websocket_handshake; apply it here too. + if (!fields::is_field_value(path)) { return -1; } + std::string s = method; s += ' '; s += path; diff --git a/test/test.cc b/test/test.cc index b1d4847622..96f695d8c8 100644 --- a/test/test.cc +++ b/test/test.cc @@ -7896,6 +7896,33 @@ TEST(ServerResponseSplittingTest, ChunkedTrailerCRLFInjection) { EXPECT_NE(std::string::npos, res.find("X-Safe: safe")); } +TEST(RequestLineInjectionTest, RejectsCRLFInTarget) { + // A well-formed target is written verbatim. + { + detail::BufferStream strm; + auto n = detail::write_request_line(strm, "GET", "/path?a=b"); + EXPECT_GT(n, 0); + EXPECT_EQ("GET /path?a=b HTTP/1.1\r\n", strm.get_buffer()); + } + + // A target carrying CR/LF must be rejected before anything reaches the wire, + // otherwise it splits the request line and injects a header or a whole + // request. This is what a decoded redirect Location ("%0D%0A") turns into + // when path encoding is disabled. + const std::string evil_targets[] = { + "/a\r\nInjected: pwned", + "/a\rInjected", + "/a\nInjected", + "/a\r\n\r\nGET /evil HTTP/1.1\r\nHost: victim\r\n\r\n", + }; + for (const auto &evil : evil_targets) { + detail::BufferStream strm; + auto n = detail::write_request_line(strm, "GET", evil); + EXPECT_LT(n, 0); + EXPECT_TRUE(strm.get_buffer().empty()); + } +} + // Sends a raw request and verifies that there isn't a crash or exception. static void test_raw_request(const std::string &req, std::string *out = nullptr) { From 8611aec7bd19a99dd170119ce4b8900bd93e47d6 Mon Sep 17 00:00:00 2001 From: Sayed Kaif Date: Tue, 14 Jul 2026 11:48:17 +0530 Subject: [PATCH 2/3] declare write_request_line in test.cc for split builds split.py strips `inline` and moves the definition into httplib.cc, so detail::write_request_line is not visible from the split httplib.h and test_split failed to compile. Re-declare it in test.cc, matching what the base64_encode and getaddrinfo_with_timeout tests already do. --- test/test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/test.cc b/test/test.cc index 96f695d8c8..76936b4940 100644 --- a/test/test.cc +++ b/test/test.cc @@ -7896,6 +7896,17 @@ TEST(ServerResponseSplittingTest, ChunkedTrailerCRLFInjection) { EXPECT_NE(std::string::npos, res.find("X-Safe: safe")); } +// Forward declaration: in split builds split.py strips `inline` and moves the +// definition into httplib.cc, so detail::write_request_line is not visible from +// the public httplib.h. Re-declaring it here lets the tests link against the +// symbol in both header-only and split builds. +namespace httplib { +namespace detail { +ssize_t write_request_line(Stream &strm, const std::string &method, + const std::string &path); +} // namespace detail +} // namespace httplib + TEST(RequestLineInjectionTest, RejectsCRLFInTarget) { // A well-formed target is written verbatim. { From bfe3265228102ac97926f36ad5537400b5391ab4 Mon Sep 17 00:00:00 2001 From: yhirose Date: Sat, 18 Jul 2026 17:48:40 -0400 Subject: [PATCH 3/3] Fail the request when write_request_line rejects the target The CR/LF guard in write_request_line returns -1, but ClientImpl::write_request ignored that return value. A rejected target therefore produced a request-line- less request (headers only) that the client silently reported as a successful send. This is the primary path reachable via a decoded redirect Location under set_path_encode(false), and it also carries the CONNECT target. Check the return value like ClientImpl::open_stream already does and fail with Error::Write. Add an end-to-end test asserting the client refuses a CR/LF target instead of putting it on the wire. --- httplib.h | 9 ++++++++- test/test.cc | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/httplib.h b/httplib.h index 531260f570..efa06839c3 100644 --- a/httplib.h +++ b/httplib.h @@ -13819,7 +13819,14 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req, } // Write request line and headers - detail::write_request_line(bstrm, req.method, path_with_query); + if (detail::write_request_line(bstrm, req.method, path_with_query) < 0) { + // A rejected target (e.g. CR/LF smuggled in via a decoded redirect + // Location under set_path_encode(false)) must fail the request cleanly + // instead of emitting a request-line-less, header-injecting request. + error = Error::Write; + output_error_log(error, &req); + return false; + } if (!detail::check_and_write_headers(bstrm, req.headers, header_writer_, error)) { output_error_log(error, &req); diff --git a/test/test.cc b/test/test.cc index 76936b4940..3ecf4e19d0 100644 --- a/test/test.cc +++ b/test/test.cc @@ -7934,6 +7934,42 @@ TEST(RequestLineInjectionTest, RejectsCRLFInTarget) { } } +TEST(RequestLineInjectionTest, ClientRejectsCRLFTargetEndToEnd) { + // End-to-end counterpart to RejectsCRLFInTarget. With path encoding disabled + // the client transmits the target verbatim, so a CR/LF-bearing target -- what + // a redirect Location "%0D%0A" decodes to -- reaches write_request. The + // client must fail cleanly with Error::Write instead of putting a + // request-line-less, header-injecting request on the wire. + Server svr; + + svr.Get("/a", [](const Request &, Response &res) { + res.set_content("ok", "text/plain"); + }); + + auto thread = std::thread([&]() { svr.listen(HOST, PORT); }); + auto se = detail::scope_exit([&] { + svr.stop(); + thread.join(); + ASSERT_FALSE(svr.is_running()); + }); + + svr.wait_until_ready(); + + { + Client cli(HOST, PORT); + cli.set_path_encode(false); + // The request is rejected before anything is written, so the connection + // stays silent and the subsequent response read would otherwise block for + // the full default read timeout. Shorten it -- the assertion is on the + // error, not the timeout. + cli.set_read_timeout(1, 0); + + auto res = cli.Get("/a\r\nInjected: pwned"); + EXPECT_FALSE(res); + EXPECT_EQ(Error::Write, res.error()); + } +} + // Sends a raw request and verifies that there isn't a crash or exception. static void test_raw_request(const std::string &req, std::string *out = nullptr) {