Просмотр исходного кода

Escape CR and LF in multipart part content types

Same header-injection vector as the name/filename fix: item.content_type
was concatenated into the part's Content-Type header unescaped, so
embedded CR/LF could inject arbitrary part headers.

Escape CR -> %0D and LF -> %0A via escape_multipart_field with a new
escape_quote = false mode. '"' is left intact since it is legal in
Content-Type values (e.g. quoted charset parameters) and appears outside
a quoted-string context here.
yhirose 1 месяц назад
Родитель
Сommit
f5c8c982df
2 измененных файлов с 26 добавлено и 6 удалено
  1. 16 4
      httplib.h
  2. 10 2
      test/test.cc

+ 16 - 4
httplib.h

@@ -1685,7 +1685,8 @@ bool is_valid_multipart_boundary(const std::string &boundary);
 // Serializer for multipart/form-data request bodies. The boundary is owned
 // by the writer so that per-part framing and the final terminator always
 // agree. Field names and filenames are escaped following the WHATWG HTML
-// standard ('"' -> %22, CR -> %0D, LF -> %0A).
+// standard ('"' -> %22, CR -> %0D, LF -> %0A); CR and LF are also escaped
+// in content types.
 class MultipartFormDataWriter {
 public:
   MultipartFormDataWriter();
@@ -8349,12 +8350,21 @@ inline bool is_multipart_boundary_chars_valid(const std::string &boundary) {
 // Escape a multipart field name/filename following the WHATWG HTML standard
 // ("escape a multipart form-data name"), which is what browsers send:
 // '"' -> %22, CR -> %0D, LF -> %0A
-inline std::string escape_multipart_field(const std::string &s) {
+// With escape_quote = false, only CR and LF are escaped; this is for header
+// values outside a quoted-string (e.g. Content-Type), where '"' is legal.
+inline std::string escape_multipart_field(const std::string &s,
+                                          bool escape_quote = true) {
   std::string result;
   result.reserve(s.size());
   for (auto c : s) {
     switch (c) {
-    case '"': result += "%22"; break;
+    case '"':
+      if (escape_quote) {
+        result += "%22";
+      } else {
+        result += c;
+      }
+      break;
     case '\r': result += "%0D"; break;
     case '\n': result += "%0A"; break;
     default: result += c; break;
@@ -8375,7 +8385,9 @@ serialize_multipart_formdata_item_begin(const T &item,
   }
   body += "\r\n";
   if (!item.content_type.empty()) {
-    body += "Content-Type: " + item.content_type + "\r\n";
+    body +=
+        "Content-Type: " + escape_multipart_field(item.content_type, false) +
+        "\r\n";
   }
   body += "\r\n";
 

+ 10 - 2
test/test.cc

@@ -6550,8 +6550,9 @@ TEST(MultipartFormDataTest, FieldEscaping) {
 
     // '"', CR and LF in names and filenames are escaped following the
     // WHATWG HTML standard, so each part still parses as a single part
-    // with no injected headers.
-    ASSERT_EQ(3U, received.size());
+    // with no injected headers. Content types get CR/LF escaped too,
+    // while '"' (legal there, e.g. quoted charset) is preserved.
+    ASSERT_EQ(4U, received.size());
 
     EXPECT_EQ("na%22me", received[0].name);
     EXPECT_EQ("quoted name", received[0].content);
@@ -6565,6 +6566,11 @@ TEST(MultipartFormDataTest, FieldEscaping) {
     EXPECT_EQ("my%0Dna%0Ame", received[2].name);
     EXPECT_EQ("my%22file.txt", received[2].filename);
     EXPECT_EQ("cr lf name", received[2].content);
+
+    EXPECT_EQ("typed", received[3].name);
+    EXPECT_EQ("text/plain; charset=\"utf-8\"%0D%0AX-Evil: 1",
+              received[3].content_type);
+    EXPECT_EQ("typed content", received[3].content);
   });
 
   auto port = svr.bind_to_any_port("localhost");
@@ -6584,6 +6590,8 @@ TEST(MultipartFormDataTest, FieldEscaping) {
       {"file", "injected", "evil\r\nContent-Type: text/evil\r\n\r\n.pdf",
        "application/octet-stream"},
       {"my\rna\nme", "cr lf name", "my\"file.txt", "text/plain"},
+      {"typed", "typed content", "",
+       "text/plain; charset=\"utf-8\"\r\nX-Evil: 1"},
   };
 
   auto res = cli.Post("/post", items);