Quellcode durchsuchen

Merge commit from fork

yhirose vor 1 Monat
Ursprung
Commit
98048a033a
2 geänderte Dateien mit 112 neuen und 13 gelöschten Zeilen
  1. 25 4
      httplib.h
  2. 87 9
      test/test.cc

+ 25 - 4
httplib.h

@@ -8457,6 +8457,23 @@ make_host_and_port_string_always_port(const std::string &host, int port) {
   return prepare_host_string(host) + ":" + std::to_string(port);
 }
 
+template <typename T>
+inline bool check_and_write_headers(Stream &strm, Headers &headers,
+                                    T header_writer, Error &error) {
+  for (const auto &h : headers) {
+    if (!detail::fields::is_field_name(h.first) ||
+        !detail::fields::is_field_value(h.second)) {
+      error = Error::InvalidHeaders;
+      return false;
+    }
+  }
+  if (header_writer(strm, headers) <= 0) {
+    error = Error::Write;
+    return false;
+  }
+  return true;
+}
+
 } // namespace detail
 
 // HTTP server implementation
@@ -8864,7 +8881,7 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
   {
     detail::BufferStream bstrm;
     if (!detail::write_response_line(bstrm, res.status)) { return false; }
-    if (!header_writer_(bstrm, res.headers)) { return false; }
+    if (header_writer_(bstrm, res.headers) <= 0) { return false; }
 
     // Flush buffer
     auto &data = bstrm.get_buffer();
@@ -10359,8 +10376,8 @@ ClientImpl::open_stream(const std::string &method, const std::string &path,
     return handle;
   }
 
-  if (!detail::write_headers(strm, req.headers)) {
-    handle.error = Error::Write;
+  if (!detail::check_and_write_headers(strm, req.headers, header_writer_,
+                                       handle.error)) {
     handle.response.reset();
     return handle;
   }
@@ -10957,7 +10974,11 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req,
 
     // Write request line and headers
     detail::write_request_line(bstrm, req.method, path_with_query);
-    header_writer_(bstrm, req.headers);
+    if (!detail::check_and_write_headers(bstrm, req.headers, header_writer_,
+                                         error)) {
+      output_error_log(error, &req);
+      return false;
+    }
 
     // Flush buffer
     auto &data = bstrm.get_buffer();

+ 87 - 9
test/test.cc

@@ -10671,6 +10671,74 @@ TEST(VulnerabilityTest, CRLFInjection) {
   }
 }
 
+TEST(VulnerabilityTest, CRLFInjectionInHeaders) {
+  auto server_thread = std::thread([] {
+    auto srv = ::socket(AF_INET, SOCK_STREAM, 0);
+    int on = 1;
+    ::setsockopt(srv, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+
+    sockaddr_in addr{};
+    addr.sin_family = AF_INET;
+    addr.sin_port = htons(PORT);
+    ::inet_pton(AF_INET, HOST, &addr.sin_addr);
+    ::bind(srv, reinterpret_cast<sockaddr *>(&addr), sizeof(addr));
+    ::listen(srv, 1);
+
+    sockaddr_in cli_addr{};
+    socklen_t cli_len = sizeof(cli_addr);
+    auto cli = ::accept(srv, reinterpret_cast<sockaddr *>(&cli_addr), &cli_len);
+
+    struct timeval tv;
+    tv.tv_sec = 1;
+    tv.tv_usec = 0;
+    ::setsockopt(cli, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+
+    std::string buf_all;
+    char buf[2048];
+    ssize_t n;
+
+    while ((n = ::recv(cli, buf, sizeof(buf), 0)) > 0) {
+      buf_all.append(buf, static_cast<size_t>(n));
+
+      size_t pos;
+      while ((pos = buf_all.find("\r\n\r\n")) != std::string::npos) {
+        auto request_block = buf_all.substr(0, pos + 4); // include separator
+
+        auto e = request_block.find("\r\n");
+        if (e != std::string::npos) {
+          auto request_line = request_block.substr(0, e);
+          std::string msg =
+              "CRLF injection detected in request line: '" + request_line + "'";
+          EXPECT_FALSE(true) << msg;
+        }
+
+        std::string resp = "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nHello";
+        ::send(cli, resp.c_str(), resp.size(), 0);
+
+        buf_all.erase(0, pos + 4);
+      }
+    }
+
+    ::close(cli);
+    ::close(srv);
+  });
+
+  std::this_thread::sleep_for(std::chrono::milliseconds(200));
+
+  auto cli = httplib::Client(HOST, PORT);
+
+  auto headers = httplib::Headers{
+      {"A", "B\r\n\r\nGET /pwned HTTP/1.1\r\nHost: 127.0.0.1:1234\r\n\r\n"},
+      {"Connection", "keep-alive"}};
+
+  auto res = cli.Get("/hi", headers);
+  EXPECT_FALSE(res);
+
+  if (res) { EXPECT_EQ(httplib::Error::InvalidHeaders, res.error()); }
+
+  server_thread.join();
+}
+
 TEST(PathParamsTest, StaticMatch) {
   const auto pattern = "/users/all";
   detail::PathParamsMatcher matcher(pattern);
@@ -11700,12 +11768,16 @@ TEST(ForwardedHeadersTest, HandlesWhitespaceAroundIPs) {
 
   svr.wait_until_ready();
 
-  Client cli(HOST, PORT);
-  auto res = cli.Get("/ip", {{"X-Forwarded-For",
-                              " 198.51.100.23 , 203.0.113.66 , 192.0.2.45 "}});
+  std::string raw_req =
+    "GET /ip HTTP/1.1\r\n"
+    "Host: localhost\r\n"
+    "X-Forwarded-For:  198.51.100.23 , 203.0.113.66 , 192.0.2.45 \r\n"
+    "Connection: close\r\n"
+    "\r\n";
 
-  ASSERT_TRUE(res);
-  EXPECT_EQ(StatusCode::OK_200, res->status);
+  std::string out;
+  ASSERT_TRUE(send_request(5, raw_req, &out));
+  EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15));
 
   // Header parser trims surrounding whitespace of the header value
   EXPECT_EQ(observed_xff, "198.51.100.23 , 203.0.113.66 , 192.0.2.45");
@@ -13086,10 +13158,16 @@ TEST(ETagTest, MalformedIfNoneMatchAndWhitespace) {
   EXPECT_EQ(200, res_bad->status);
 
   // Whitespace-only header value should be considered invalid / non-matching
-  Headers h_space = {{"If-None-Match", "   "}};
-  auto res_space = cli.Get("/static/etag_malformed.txt", h_space);
-  ASSERT_TRUE(res_space);
-  EXPECT_EQ(200, res_space->status);
+  std::string raw_req =
+    "GET /static/etag_malformed.txt HTTP/1.1\r\n"
+    "Host: localhost\r\n"
+    "If-None-Match:   \r\n"
+    "Connection: close\r\n"
+    "\r\n";
+
+  std::string out;
+  ASSERT_TRUE(send_request(5, raw_req, &out));
+  EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15));
 
   svr.stop();
   t.join();