Browse Source

Bound the bytes around chunk data in chunked transfer coding

The payload limit counts only chunk data, and a chunk-size line is only
bounded by the line length limit, so a peer could send a few bytes of
data with any amount of padding, leading zeros and chunk extensions
around them. Count those bytes as Go does: each chunk may spend 16
bytes plus twice its data on its size line and the CRLF after it, which
still lets a stream send a byte at a time, and the excess may build up
to CPPHTTPLIB_HEADER_MAX_LENGTH before the read fails. This covers
request bodies on the server and response bodies on the client.
yhirose 1 day ago
parent
commit
247a2abe4a
2 changed files with 58 additions and 0 deletions
  1. 10 0
      httplib.h
  2. 48 0
      test/test.cc

+ 10 - 0
httplib.h

@@ -4019,6 +4019,7 @@ struct ChunkedDecoder {
   char line_buf[64];
   size_t last_chunk_total = 0;
   size_t last_chunk_offset = 0;
+  size_t overhead = 0;
 
   explicit ChunkedDecoder(Stream &s);
 
@@ -16113,6 +16114,15 @@ inline ssize_t ChunkedDecoder::read_payload(char *buf, size_t len,
       if (!is_space_or_tab(*p) && !fields::is_field_vchar(*p)) { return -1; }
     }
 
+    // The bytes around the data are no payload, so they are counted on their
+    // own, as Go does: each chunk may spend 16 bytes and twice its data on
+    // them, which lets a stream send a byte at a time, and what goes past that
+    // in padding, leading zeros and extensions builds up to one header field.
+    overhead += lr.size() + 2;
+    overhead = overhead > 16 ? overhead - 16 : 0;
+    overhead = chunk_len > overhead / 2 ? 0 : overhead - 2 * chunk_len;
+    if (overhead > CPPHTTPLIB_HEADER_MAX_LENGTH) { return -1; }
+
     if (chunk_len == 0) {
       chunk_remaining = 0;
       finished = true;

+ 48 - 0
test/test.cc

@@ -8073,6 +8073,23 @@ TEST_F(ServerTest, RejectsControlCharacterInChunkExtension) {
             "b\r\ndech\r\nf\r\nunked post body\r\n0\r\n\r\n");
 }
 
+// The bytes around chunk data are no payload, so they cannot ride on the
+// payload limit
+TEST_F(ServerTest, RejectsChunkOverheadPastItsLimit) {
+  auto pad = std::string(CPPHTTPLIB_HEADER_MAX_LENGTH / 2, 'x');
+  auto zeros = std::string(CPPHTTPLIB_HEADER_MAX_LENGTH / 2, '0');
+  auto spaces = std::string(CPPHTTPLIB_HEADER_MAX_LENGTH / 2, ' ');
+  for (const auto &size_line : {"1;" + pad, zeros + "1", "1" + spaces}) {
+    SCOPED_TRACE(size_line.substr(0, 2));
+    std::string body;
+    for (auto c : std::string("dechunked post body")) {
+      body += size_line + "\r\n" + c + "\r\n";
+    }
+    body += "0\r\n\r\n";
+    expect_chunked_body_rejected(cli_, body.c_str());
+  }
+}
+
 TEST_F(ServerTest, AcceptsChunkExtension) {
   expect_chunked_body_status(cli_,
                              "4;name=value\r\ndech\r\n"
@@ -11125,6 +11142,37 @@ TEST(HeadersOrderTest, SentFieldsKeepTheirOrder) {
   EXPECT_EQ("second=2", res->get_header_value("Set-Cookie", "", 1));
 }
 
+// A stream may send a byte at a time with a small extension on each chunk,
+// however long it runs
+TEST(ChunkOverheadTest, AcceptsSmallOverheadOnEveryChunk) {
+  Server svr;
+  svr.Post("/count", [](const Request &req, Response &res) {
+    res.set_content(std::to_string(req.body.size()), "text/plain");
+  });
+  auto port = svr.bind_to_any_port(HOST);
+  ASSERT_LT(0, port);
+  thread t = thread([&] { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  std::string req = std::string("POST /count HTTP/1.1\r\nHost: ") + HOST +
+                    "\r\nTransfer-Encoding: chunked\r\n"
+                    "Connection: close\r\n\r\n";
+  for (auto i = 0; i < 4096; i++) {
+    req += "1;a=b\r\nx\r\n";
+  }
+  req += "0\r\n\r\n";
+
+  std::string res;
+  ASSERT_TRUE(send_request(5, req, &res, port));
+  EXPECT_EQ(0u, res.find("HTTP/1.1 200 OK\r\n"));
+  EXPECT_EQ("4096", res.substr(res.size() - 4));
+}
+
 TEST(ServerResponseSplittingTest, ChunkedTrailerCRLFInjection) {
   Server svr;
   svr.Get("/injected-trailer", [&](const Request & /*req*/, Response &res) {