Fixed issues with multipart parser

- Index was not checked against size before dereference. Which cased to
  override memory.
- Header without colon could put parser into invalid state. Now it will
  return with error.
- Content after boundary was not correctly discarded.
- Parser did not check body for final boudary. Now missing final
  boundary will return with error.

Tested:
- Tested that payload with header without colon doesn't cause memory
  corruption anymore.

Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I12f496ab5f53e6c088cdfdf2e96be636d66f7c7f

1 files changed, 183 insertions, 9 deletions

diff --git a/test/include/multipart_test.cpp b/test/include/multipart_test.cpp
index 7ad7d98f89..f29bcfe5c8 100644
--- a/test/include/multipart_test.cpp
+++ b/test/include/multipart_test.cpp
@@ -83,11 +83,8 @@ TEST_F(MultipartTest, TestBadMultipartParser1)
 
     crow::Request reqIn(req, ec);
     ParserError rc = parser.parse(reqIn);
-    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
 
-    EXPECT_EQ(parser.boundary,
-              "\r\n-----------------------------d74496d66958873e");
-    EXPECT_EQ(parser.mime_fields.size(), 1);
+    EXPECT_EQ(rc, ParserError::ERROR_UNEXPECTED_END_OF_INPUT);
 }
 
 TEST_F(MultipartTest, TestBadMultipartParser2)
@@ -103,11 +100,8 @@ TEST_F(MultipartTest, TestBadMultipartParser2)
 
     crow::Request reqIn(req, ec);
     ParserError rc = parser.parse(reqIn);
-    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
 
-    EXPECT_EQ(parser.boundary,
-              "\r\n-----------------------------d74496d66958873e");
-    EXPECT_EQ(parser.mime_fields.size(), 1);
+    EXPECT_EQ(rc, ParserError::ERROR_UNEXPECTED_END_OF_INPUT);
 }
 
 TEST_F(MultipartTest, TestErrorBoundaryFormat)
@@ -253,4 +247,184 @@ TEST_F(MultipartTest, TestErrorHeaderEnding)
     crow::Request reqIn(req, ec);
     EXPECT_EQ(parser.parse(reqIn), ParserError::ERROR_HEADER_ENDING);
 }
-} // namespace
\ No newline at end of file
+
+TEST_F(MultipartTest, TestGoodMultipartParserMultipleHeaders)
+{
+    req.set("Content-Type",
+            "multipart/form-data; "
+            "boundary=---------------------------d74496d66958873e");
+
+    req.body() = "-----------------------------d74496d66958873e\r\n"
+                 "Content-Disposition: form-data; name=\"Test1\"\r\n"
+                 "Other-Header: value=\"v1\"\r\n"
+                 "\r\n"
+                 "Data1\r\n"
+                 "-----------------------------d74496d66958873e--";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
+
+    EXPECT_EQ(parser.boundary,
+              "\r\n-----------------------------d74496d66958873e");
+    ASSERT_EQ(parser.mime_fields.size(), 1);
+
+    EXPECT_EQ(parser.mime_fields[0].fields.at("Content-Disposition"),
+              "form-data; name=\"Test1\"");
+    EXPECT_EQ(parser.mime_fields[0].fields.at("Other-Header"), "value=\"v1\"");
+    EXPECT_EQ(parser.mime_fields[0].content, "Data1");
+}
+
+TEST_F(MultipartTest, TestErrorHeaderWithoutColon)
+{
+    req.set("Content-Type", "multipart/form-data; "
+                            "boundary=--end");
+
+    req.body() = "----end\r\n"
+                 "abc\r\n"
+                 "\r\n"
+                 "Data1\r\n"
+                 "----end--\r\n";
+
+    crow::Request reqIn(req, ec);
+    EXPECT_EQ(parser.parse(reqIn), ParserError::ERROR_UNEXPECTED_END_OF_HEADER);
+}
+
+TEST_F(MultipartTest, TestUnknownHeaderIsCorrectlyParsed)
+{
+    req.set("Content-Type", "multipart/form-data; "
+                            "boundary=--end");
+
+    req.body() =
+        "----end\r\n"
+        "t-DiPpcccc:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccgcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccaaaaaa\r\n"
+        "\r\n"
+        "Data1\r\n"
+        "----end--\r\n";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
+
+    EXPECT_EQ(parser.boundary, "\r\n----end");
+    ASSERT_EQ(parser.mime_fields.size(), 1);
+
+    EXPECT_EQ(
+        parser.mime_fields[0].fields.at("t-DiPpcccc"),
+        "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccgcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccaaaaaa");
+    EXPECT_EQ(parser.mime_fields[0].content, "Data1");
+}
+
+TEST_F(MultipartTest, TestErrorMissingSeparatorBetweenMimeFieldsAndData)
+{
+    req.set(
+        "Content-Type",
+        "multipart/form-data; boundary=---------------------------d74496d66958873e");
+
+    req.body() =
+        "-----------------------------d74496d66958873e\r\n"
+        "t-DiPpcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccgcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccaaaaaa\r\n"
+        "Data1"
+        "-----------------------------d74496d66958873e--";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    EXPECT_EQ(rc, ParserError::ERROR_UNEXPECTED_END_OF_HEADER);
+}
+
+TEST_F(MultipartTest, TestDataWithoutMimeFields)
+{
+    req.set(
+        "Content-Type",
+        "multipart/form-data; boundary=---------------------------d74496d66958873e");
+
+    req.body() = "-----------------------------d74496d66958873e\r\n"
+                 "\r\n"
+                 "Data1\r\n"
+                 "-----------------------------d74496d66958873e--";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
+
+    EXPECT_EQ(parser.boundary,
+              "\r\n-----------------------------d74496d66958873e");
+    ASSERT_EQ(parser.mime_fields.size(), 1);
+
+    EXPECT_EQ(std::distance(parser.mime_fields[0].fields.begin(),
+                            parser.mime_fields[0].fields.end()),
+              0);
+    EXPECT_EQ(parser.mime_fields[0].content, "Data1");
+}
+
+TEST_F(MultipartTest, TestErrorMissingFinalBoundry)
+{
+    req.set("Content-Type", "multipart/form-data; boundary=--XX");
+
+    req.body() =
+        "----XX\r\n"
+        "Content-Disposition: form-data; name=\"Test2\"\r\n\r\n"
+        "t-DiPpccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccAAAAAAAAAAAAAAABCDz\r\n"
+        "\335\r\n\r\n";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    EXPECT_EQ(rc, ParserError::ERROR_UNEXPECTED_END_OF_INPUT);
+}
+
+TEST_F(MultipartTest, TestIgnoreDataAfterFinalBoundary)
+{
+    req.set("Content-Type", "multipart/form-data; boundary=--XX");
+
+    req.body() = "----XX\r\n"
+                 "Content-Disposition: form-data; name=\"Test1\"\r\n\r\n"
+                 "Data1\r\n"
+                 "----XX--\r\n"
+                 "Content-Disposition: form-data; name=\"Test2\"\r\n\r\n"
+                 "Data2\r\n"
+                 "----XX--\r\n";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
+
+    EXPECT_EQ(parser.boundary, "\r\n----XX");
+    EXPECT_EQ(parser.mime_fields.size(), 1);
+
+    EXPECT_EQ(parser.mime_fields[0].fields.at("Content-Disposition"),
+              "form-data; name=\"Test1\"");
+    EXPECT_EQ(parser.mime_fields[0].content, "Data1");
+}
+
+TEST_F(MultipartTest, TestFinalBoundaryIsCorrectlyRecognized)
+{
+    req.set("Content-Type", "multipart/form-data; boundary=--XX");
+
+    req.body() = "----XX\r\n"
+                 "Content-Disposition: form-data; name=\"Test1\"\r\n\r\n"
+                 "Data1\r\n"
+                 "----XX-abc-\r\n"
+                 "StillData1\r\n"
+                 "----XX--\r\n";
+
+    crow::Request reqIn(req, ec);
+    ParserError rc = parser.parse(reqIn);
+
+    ASSERT_EQ(rc, ParserError::PARSER_SUCCESS);
+
+    EXPECT_EQ(parser.boundary, "\r\n----XX");
+    EXPECT_EQ(parser.mime_fields.size(), 1);
+
+    EXPECT_EQ(parser.mime_fields[0].fields.at("Content-Disposition"),
+              "form-data; name=\"Test1\"");
+    EXPECT_EQ(parser.mime_fields[0].content, "Data1\r\n"
+                                             "----XX-abc-\r\n"
+                                             "StillData1");
+}
+
+} // namespace