Bladeren bron

Cap the received multipart boundary at RFC 2046's 70 characters (#2565)

parse_multipart_boundary only rejected an empty boundary, so a request could
declare one as long as a header line is allowed to be. A stock server accepts
up to 8146 bytes there, which is what CPPHTTPLIB_HEADER_MAX_LENGTH leaves after
"Content-Type: multipart/form-data; boundary=".

FormDataParser searches the body for "--" + boundary + CRLF with a plain
substring scan. buf_find scans for that delimiter's first byte, always '-', and
at every position that matches calls start_with, which compares until the first
mismatch. A body of '-' makes every position a candidate, and a boundary of '-'
makes each candidate compare the whole delimiter before failing at the CRLF. The
worst case is the product of the body length and the boundary length, and only
the first factor was bounded.

Measured by driving the parser directly in 16 KB reads, Apple clang 17 at
-O2 -DNDEBUG, best of three runs on an otherwise idle machine. 100 MB of '-',
the default payload limit, costs 2.59 s of CPU with a 70 byte boundary and
281.83 s with an 8147 byte one, a factor of 109. The same shape shows at 8 MB:
0.211 s, 3.081 s, 11.359 s and 22.091 s for boundaries of 70, 1024, 4096 and
8147 bytes.

RFC 2046 5.1.1 caps a boundary at 70 characters, so honoring that limit bounds
the multiplier too. The limit applies to the value after unquoting, so a quoted
70 character boundary stays valid. Only the server receive path parses a
boundary out of a Content-Type, so what clients may send is unaffected, and the
boundaries the library generates itself are 45 characters.
yhirose 3 dagen geleden
bovenliggende
commit
19352ae929
2 gewijzigde bestanden met toevoegingen van 84 en 1 verwijderingen
  1. 5 1
      httplib.h
  2. 79 0
      test/test.cc

+ 5 - 1
httplib.h

@@ -8659,7 +8659,11 @@ inline bool parse_multipart_boundary(const std::string &content_type,
   auto it = params.find("boundary");
   if (it == params.end()) { return false; }
   boundary = it->second;
-  return !boundary.empty();
+  // RFC 2046 5.1.1 caps a boundary at 70 characters. The parser scans the body
+  // for "--" + boundary, so a body crafted to repeat that delimiter's leading
+  // bytes costs a nearly full comparison at nearly every position: the
+  // boundary's length multiplies the worst-case cost of scanning a body.
+  return !boundary.empty() && boundary.size() <= 70;
 }
 
 inline void parse_disposition_params(const std::string &s, Params &params) {

+ 79 - 0
test/test.cc

@@ -1510,6 +1510,37 @@ TEST(ParseMultipartBoundaryTest, ValueWithQuotesAndCharset) {
   EXPECT_EQ(boundary, "cpp-httplib-multipart-data");
 }
 
+TEST(ParseMultipartBoundaryTest, LongestAllowedValue) {
+  const string boundary(70, 'a');
+  string content_type = "multipart/form-data; boundary=" + boundary;
+  string parsed;
+  auto ret = detail::parse_multipart_boundary(content_type, parsed);
+  EXPECT_TRUE(ret);
+  EXPECT_EQ(parsed, boundary);
+}
+
+TEST(ParseMultipartBoundaryTest, ValueLongerThanRFCLimit) {
+  // RFC 2046 5.1.1 caps a boundary at 70 characters.
+  string content_type = "multipart/form-data; boundary=" + string(71, 'a');
+  string parsed;
+  auto ret = detail::parse_multipart_boundary(content_type, parsed);
+  EXPECT_FALSE(ret);
+}
+
+TEST(ParseMultipartBoundaryTest, QuotedValueIsMeasuredAfterUnquoting) {
+  // The limit applies to the unquoted value, so a quoted 70 character boundary
+  // is 72 characters on the wire and still valid.
+  const string boundary(70, 'a');
+  string content_type = "multipart/form-data; boundary=\"" + boundary + "\"";
+  string parsed;
+  EXPECT_TRUE(detail::parse_multipart_boundary(content_type, parsed));
+  EXPECT_EQ(parsed, boundary);
+
+  content_type = "multipart/form-data; boundary=\"" + string(71, 'a') + "\"";
+  parsed.clear();
+  EXPECT_FALSE(detail::parse_multipart_boundary(content_type, parsed));
+}
+
 TEST(GetHeaderValueTest, DefaultValue) {
   Headers headers = {{"Dummy", "Dummy"}};
   auto val = detail::get_header_value(headers, "Content-Type", "text/plain", 0);
@@ -15508,6 +15539,54 @@ TEST(MultipartFormDataTest, InitialBoundarySplitAfterLongPreamble) {
                             "\r\n--zzzz--\r\n");
 }
 
+TEST(MultipartFormDataTest, BoundaryLengthLimitIsEnforced) {
+  // The received boundary was only checked for being non-empty, so it could be
+  // as long as a header is allowed to be. The parser scans the body for
+  // "--" + boundary, so a body crafted to repeat that delimiter's leading bytes
+  // costs a nearly full comparison at nearly every position, making the
+  // boundary's length a multiplier on the worst-case parsing cost. RFC 2046
+  // 5.1.1 caps a boundary at 70 characters; honoring that caps the multiplier.
+  Server svr;
+  svr.Post("/post", [](const Request &req, Response &res) {
+    EXPECT_EQ(1u, req.form.fields.size());
+    EXPECT_EQ("text1", req.form.get_field("text1"));
+    res.set_content("ok", "text/plain");
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  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();
+
+  Client cli(HOST, port);
+
+  auto post_with_boundary_of_length = [&](size_t len) {
+    const std::string boundary(len, 'a');
+    const std::string body =
+        "--" + boundary +
+        "\r\n"
+        "Content-Disposition: form-data; name=\"text1\"\r\n"
+        "\r\n"
+        "text1"
+        "\r\n--" +
+        boundary + "--\r\n";
+    return cli.Post("/post", body, "multipart/form-data; boundary=" + boundary);
+  };
+
+  auto res = post_with_boundary_of_length(70);
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::OK_200, res->status);
+
+  res = post_with_boundary_of_length(71);
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::BadRequest_400, res->status);
+}
+
 TEST(MakeFileBodyTest, Basic) {
   const std::string file_content(4096, 'Z');
   const std::string tmp_path = "./httplib_test_make_file_body.bin";