escape quoted-string auth-params in make_digest_authentication_header (#2597)

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<std::string, std::string> &auth,
                                    bool is_proxy) {
@@ -10614,7 +10627,13 @@ inline std::pair<std::string, std::string> 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<std::string, std::string> make_digest_authentication_header(
 
   auto opaque = (auth.find("opaque") != auth.end()) ? auth.at("opaque") : "";
 
-  auto field = "Digest username=\"" + username + "\", realm=\"" +
+  // Every value placed inside a quoted-string is escaped so a '"' in it cannot
-               auth.at("realm") + "\", nonce=\"" + auth.at("nonce") +
+  // close the string early. realm, nonce and opaque come straight from the
-               "\", uri=\"" + req.path + "\", algorithm=" + algo +
+  // server's challenge (parse_www_authenticate() already de-escaped them), so
-               (qop.empty() ? ", response=\""
+  // without this a crafted challenge injects extra auth-params into the header.
-                            : ", qop=" + qop + ", nc=" + nc + ", cnonce=\"" +
+  auto field =
-                                  cnonce + "\", response=\"") +
+      "Digest username=\"" + detail::escape_quoted_pairs(username) +
-               response + "\"" +
+      "\", realm=\"" + detail::escape_quoted_pairs(auth.at("realm")) +
-               (opaque.empty() ? "" : ", opaque=\"" + opaque + "\"");
+      "\", 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);

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<int> 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) {