From fbb031ed8504a0e48d9ca63caecc0614c296f755 Mon Sep 17 00:00:00 2001 From: yhirose Date: Sun, 10 May 2026 12:40:33 +0900 Subject: [PATCH] Stop percent-decoding HTTP request header values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_header() applied decode_path_component() to every header value except Location and Referer, after is_field_value() validation. Wire sequences like %0D%0A passed the check and expanded into literal CR/LF inside stored values, enabling response splitting, log injection, and proxy smuggling. %3D/%2C/%3B also flipped Cookie and X-Forwarded-For boundaries against WAFs inspecting the wire form. RFC 9110 §5.5 specifies header values as opaque octets. Drop the decoding and the Location/Referer special case (originally workarounds for the same auto-decode misbehavior; redundant once decoding stops). Applications that need URI semantics should call decode_uri_component() or decode_path_component() on the result explicitly. Add regression tests covering CRLF injection, %3D/%2C/%3B boundary characters, UTF-8 and %uXXXX sequences, browser-style Referer URLs containing %0A (issue #2033), and the explicit-decode migration pattern. --- httplib.h | 11 +++-- test/test.cc | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/httplib.h b/httplib.h index 521b4f81..ec0ec592 100644 --- a/httplib.h +++ b/httplib.h @@ -5016,12 +5016,11 @@ inline bool parse_header(const char *beg, const char *end, T fn) { if (!detail::fields::is_field_value(val)) { return false; } - if (case_ignore::equal(key, "Location") || - case_ignore::equal(key, "Referer")) { - fn(key, val); - } else { - fn(key, decode_path_component(val)); - } + // RFC 9110 §5.5: header field values are opaque octets and MUST NOT be + // percent-decoded by the recipient. Applications that need to interpret a + // value as a URI component should call httplib::decode_uri_component() + // (or decode_path_component()) explicitly. + fn(key, val); return true; } diff --git a/test/test.cc b/test/test.cc index 4a2ad50e..9d6fb62a 100644 --- a/test/test.cc +++ b/test/test.cc @@ -7441,6 +7441,122 @@ TEST(ServerRequestParsingTest, EmptyFieldValue) { EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15)); } +TEST(ServerRequestParsingTest, HeaderValueNotPercentDecoded) { + Server svr; + std::string x_custom; + std::string cookie; + std::string xff; + std::string x_unicode; + std::string x_iis; + + svr.Get("/check", [&](const Request &req, Response &res) { + x_custom = req.get_header_value("X-Custom"); + cookie = req.get_header_value("Cookie"); + xff = req.get_header_value("X-Forwarded-For"); + x_unicode = req.get_header_value("X-Unicode"); + x_iis = req.get_header_value("X-IIS"); + 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(); + + const std::string req = "GET /check HTTP/1.1\r\n" + "Host: localhost\r\n" + "X-Custom: a%0D%0AInjected: b\r\n" + "Cookie: session%3Dvictim%3B%20admin%3Dyes\r\n" + "X-Forwarded-For: 1.2.3.4%2C5.6.7.8\r\n" + "X-Unicode: %E3%81%82\r\n" + "X-IIS: %u00E9\r\n" + "Connection: close\r\n" + "\r\n"; + + std::string res; + ASSERT_TRUE(send_request(5, req, &res)); + EXPECT_EQ("HTTP/1.1 200 OK", res.substr(0, 15)); + + // Every value must be returned verbatim (wire form), with no decoding. + EXPECT_EQ("a%0D%0AInjected: b", x_custom); + EXPECT_EQ("session%3Dvictim%3B%20admin%3Dyes", cookie); + EXPECT_EQ("1.2.3.4%2C5.6.7.8", xff); + EXPECT_EQ("%E3%81%82", x_unicode); + EXPECT_EQ("%u00E9", x_iis); +} + +// Applications that previously relied on automatic percent-decoding can +// reproduce the old behavior by explicitly calling decode_path_component() +// or, for RFC 3986 conformance, decode_uri_component(). +TEST(ServerRequestParsingTest, HeaderValueExplicitDecodingByApplication) { + Server svr; + std::string decoded; + + svr.Get("/check", [&](const Request &req, Response &res) { + decoded = decode_uri_component(req.get_header_value("X-Custom")); + 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(); + + const std::string req = "GET /check HTTP/1.1\r\n" + "Host: localhost\r\n" + "X-Custom: hello%20world\r\n" + "Connection: close\r\n" + "\r\n"; + + std::string res; + ASSERT_TRUE(send_request(5, req, &res)); + EXPECT_EQ("HTTP/1.1 200 OK", res.substr(0, 15)); + EXPECT_EQ("hello world", decoded); +} + +// Regression test for #2033. Browsers send Referer values that include +// percent-encoded characters such as %0A inside the URL. Decoding the +// header value would either trip the post-decode CR/LF/NUL guard (the +// original bug, returning 400) or, after that guard was relaxed, silently +// store a literal LF — both unacceptable. The wire form must round-trip. +TEST(ServerRequestParsingTest, RefererWithPercentEncodedNewline) { + Server svr; + std::string referer; + + svr.Get("/check", [&](const Request &req, Response &res) { + referer = req.get_header_value("Referer"); + 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(); + + const std::string req = "GET /check HTTP/1.1\r\n" + "Host: localhost\r\n" + "Referer: http://localhost:1111/?q=Hello%0A\r\n" + "Connection: close\r\n" + "\r\n"; + + std::string res; + ASSERT_TRUE(send_request(5, req, &res)); + EXPECT_EQ("HTTP/1.1 200 OK", res.substr(0, 15)); + EXPECT_EQ("http://localhost:1111/?q=Hello%0A", referer); +} + TEST(ServerStopTest, StopServerWithChunkedTransmission) { Server svr;