diff --git a/lib/src/HttpRequestImpl.cc b/lib/src/HttpRequestImpl.cc index c0b7f64706..20054c041a 100644 --- a/lib/src/HttpRequestImpl.cc +++ b/lib/src/HttpRequestImpl.cc @@ -78,9 +78,9 @@ void HttpRequestImpl::parseParameters() const if (!input.empty()) { std::string_view::size_type pos = 0; - while ((input[pos] == '?' || - isspace(static_cast(input[pos]))) && - pos < input.length()) + while (pos < input.length() && + (input[pos] == '?' || + isspace(static_cast(input[pos])))) { ++pos; } @@ -139,9 +139,9 @@ void HttpRequestImpl::parseParameters() const type.find("application/x-www-form-urlencoded") != std::string::npos) { std::string_view::size_type pos = 0; - while ((input[pos] == '?' || - isspace(static_cast(input[pos]))) && - pos < input.length()) + while (pos < input.length() && + (input[pos] == '?' || + isspace(static_cast(input[pos])))) { ++pos; } diff --git a/lib/src/MultiPart.cc b/lib/src/MultiPart.cc index 3b11af9170..eea95bbd2e 100644 --- a/lib/src/MultiPart.cc +++ b/lib/src/MultiPart.cc @@ -180,6 +180,8 @@ int MultiPartParser::parse(const HttpRequestPtr &req, std::string_view boundary{boundaryData, boundaryLen}; if (boundary.size() > 2 && boundary[0] == '\"') boundary = boundary.substr(1, boundary.size() - 2); + if (boundary.empty()) + return -1; std::string_view::size_type pos1, pos2; pos1 = 0; auto content = static_cast(req.get())->bodyView(); @@ -190,11 +192,15 @@ int MultiPartParser::parse(const HttpRequestPtr &req, if (pos1 == std::string_view::npos) break; pos1 += boundary.length(); + if (content.size() - pos1 < 2) + return -1; if (content[pos1] == '\r' && content[pos1 + 1] == '\n') pos1 += 2; pos2 = content.find(boundary, pos1); if (pos2 == std::string_view::npos) break; + if (pos2 - pos1 < 4) + return -1; bool flag = false; if (content[pos2 - 4] == '\r' && content[pos2 - 3] == '\n' && content[pos2 - 2] == '-' && content[pos2 - 1] == '-') diff --git a/lib/tests/unittests/MultiPartParserTest.cc b/lib/tests/unittests/MultiPartParserTest.cc index 0641203ada..07b61f8398 100644 --- a/lib/tests/unittests/MultiPartParserTest.cc +++ b/lib/tests/unittests/MultiPartParserTest.cc @@ -84,6 +84,23 @@ DROGON_TEST(MultiPartParserDashBoundary) CHECK(parser.getParameters().at("key1") == "value1"); } +DROGON_TEST(MultiPartParserRejectsNearbyBoundary) +{ + auto req = drogon::HttpRequest::newHttpRequest(); + req->setMethod(drogon::Post); + req->addHeader("content-type", "multipart/form-data; boundary=A"); + // The second boundary starts only three bytes into the body. + req->setBody(std::string("A\r\nA") + std::string(100, 'x')); + + drogon::MultiPartParser parser; + CHECK(parser.parse(req) == -1); + + // A boundary at the end leaves no bytes for the CRLF check. + req->setBody("A"); + drogon::MultiPartParser parser2; + CHECK(parser2.parse(req) == -1); +} + DROGON_TEST(MultiPartDispositionParametersAreOrderIndependent) { static const std::string contentType =