From dd71728110fce48aab007569f977d9fc7ba3e9ac Mon Sep 17 00:00:00 2001 From: metsw24-max Date: Sat, 3 Oct 2026 06:59:35 +0530 Subject: [PATCH] escape quoted-string auth-params in make_digest_authentication_header (#2597) --- httplib.h | 46 +++++++++++++++++++++++++++++++++++++--------- test/test.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/httplib.h b/httplib.h index 8ff4da18..b2681bba 100644 --- a/httplib.h +++ b/httplib.h @@ -10111,6 +10111,19 @@ inline std::string unescape_quoted_pairs(const std::string &s) { return out; } +// Inverse of unescape_quoted_pairs: prepares a value to sit inside a +// quoted-string. RFC 9110 §5.6.4 requires a literal '\' or '"' to be sent as a +// quoted-pair, so the recipient recovers the original value. +inline std::string escape_quoted_pairs(const std::string &s) { + std::string out; + out.reserve(s.size()); + for (auto c : s) { + if (c == '\\' || c == '"') { out += '\\'; } + out += c; + } + return out; +} + inline bool parse_www_authenticate(const Response &res, std::map &auth, bool is_proxy) { @@ -10614,7 +10627,13 @@ inline std::pair make_digest_authentication_header( } std::string algo = "MD5"; - if (auth.find("algorithm") != auth.end()) { algo = auth.at("algorithm"); } + if (auth.find("algorithm") != auth.end()) { + // algorithm is an unquoted token (RFC 7616 §3.4). A server value that is + // not a token would otherwise be emitted verbatim and could carry commas + // or quotes that inject further auth-params into the header below. + const auto &a = auth.at("algorithm"); + if (fields::is_token(a)) { algo = a; } + } std::string response; { @@ -10637,14 +10656,23 @@ inline std::pair make_digest_authentication_header( auto opaque = (auth.find("opaque") != auth.end()) ? auth.at("opaque") : ""; - auto field = "Digest username=\"" + username + "\", realm=\"" + - auth.at("realm") + "\", nonce=\"" + auth.at("nonce") + - "\", uri=\"" + req.path + "\", algorithm=" + algo + - (qop.empty() ? ", response=\"" - : ", qop=" + qop + ", nc=" + nc + ", cnonce=\"" + - cnonce + "\", response=\"") + - response + "\"" + - (opaque.empty() ? "" : ", opaque=\"" + opaque + "\""); + // Every value placed inside a quoted-string is escaped so a '"' in it cannot + // close the string early. realm, nonce and opaque come straight from the + // server's challenge (parse_www_authenticate() already de-escaped them), so + // without this a crafted challenge injects extra auth-params into the header. + auto field = + "Digest username=\"" + detail::escape_quoted_pairs(username) + + "\", realm=\"" + detail::escape_quoted_pairs(auth.at("realm")) + + "\", nonce=\"" + detail::escape_quoted_pairs(auth.at("nonce")) + + "\", uri=\"" + detail::escape_quoted_pairs(req.path) + + "\", algorithm=" + algo + + (qop.empty() ? ", response=\"" + : ", qop=" + qop + ", nc=" + nc + ", cnonce=\"" + cnonce + + "\", response=\"") + + response + "\"" + + (opaque.empty() + ? "" + : ", opaque=\"" + detail::escape_quoted_pairs(opaque) + "\""); auto key = is_proxy ? "Proxy-Authorization" : "Authorization"; return std::make_pair(key, field); diff --git a/test/test.cc b/test/test.cc index 79357a47..3e9fdbf8 100644 --- a/test/test.cc +++ b/test/test.cc @@ -3247,6 +3247,53 @@ TEST(DigestAuthTest, ChallengeMissingRealmDoesNotCrash) { run_digest_challenge_missing_field_test("Digest nonce=\"n\", qop=\"auth\""); } +// A hostile server can put a '"' in realm/nonce/opaque, or a non-token +// algorithm, in its challenge. parse_www_authenticate() de-escapes quoted-pairs +// when storing the values, so the header builder has to re-escape them (and +// keep algorithm a bare token); otherwise the value breaks out of its +// quoted-string and injects extra auth-params into the client's Authorization. +TEST(DigestAuthTest, EscapesInjectedAuthParams) { + std::atomic hits{0}; + std::string authorization; + + Server svr; + svr.Get("/x", [&](const Request &req, Response &res) { + if (++hits == 1) { + res.status = StatusCode::Unauthorized_401; + // On the wire the quotes embedded in the values are backslash-escaped. + res.set_header("WWW-Authenticate", + "Digest realm=\"testrealm\", " + "nonce=\"n\\\"; injected=\\\"x\", " + "algorithm=\"MD5, injected2=\\\"y\\\"\", qop=\"auth\""); + } else { + authorization = req.get_header_value("Authorization"); + res.set_content("ok", "text/plain"); + } + }); + + auto port = svr.bind_to_any_port(HOST); + std::thread t([&]() { svr.listen_after_bind(); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + }); + svr.wait_until_ready(); + + Client cli(HOST, port); + cli.set_digest_auth("hello", "world"); + auto res = cli.Get("/x"); + ASSERT_TRUE(res) << "Error: " << to_string(res.error()); + EXPECT_EQ(2, hits.load()); + + EXPECT_EQ(0u, authorization.rfind("Digest ", 0)); + // The nonce (de-escaped to n"; injected="x ) must be re-escaped so it stays + // inside its quoted-string rather than starting an "injected" auth-param. + EXPECT_NE(std::string::npos, + authorization.find("nonce=\"n\\\"; injected=\\\"x\"")); + // A non-token algorithm falls back to a bare MD5 token, dropping the payload. + EXPECT_EQ(std::string::npos, authorization.find("injected2")); +} + #endif TEST(SpecifyServerIPAddressTest, AnotherHostname_Online) {