From 852a37474892996c2a6968380f07eb0fe200f9fa Mon Sep 17 00:00:00 2001 From: Omkar Jadhav Date: Thu, 3 Sep 2020 22:47:52 +0530 Subject: [PATCH] Fix server crash caused due to regex complexity while matching headers. (#632) * Fix parsing to parse query string with single space char. When passed ' ' as a query string, the server crashes cause of illegal memory access done in httplib::detail::split. Have added checks to make sure the split function has a valid string with length > 0. * Fix parsing to parse query string with single space char. * Fix server crash caused due to regex complexity while matching headers. While parsing content-type header in multipart form request the server crashes due to the exhaustion of max iterations performed while matching the input string with content-type regex. Have removed the regex which might use backtracking while matching and replaced it with manual string processing. Have added tests as well. * Remove magic number Co-authored-by: Ivan Fefer Co-authored-by: yhirose Co-authored-by: Ivan Fefer --- httplib.h | 28 ++++++++++++++++++++++++---- test/test.cc | 27 ++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/httplib.h b/httplib.h index b78a650..a6ab8e3 100644 --- a/httplib.h +++ b/httplib.h @@ -1452,6 +1452,13 @@ inline std::pair trim(const char *b, const char *e, int left, return std::make_pair(left, right); } +inline void trim(std::string &s) { + auto is_not_space = [](int ch) { return !std::isspace(ch); }; + s.erase(s.begin(), std::find_if(s.begin(), s.end(), is_not_space)); + s.erase(std::find_if(s.rbegin(), s.rend(), is_not_space).base(), s.end()); +} + + template void split(const char *b, const char *e, char d, Fn fn) { int i = 0; int beg = 0; @@ -2828,6 +2835,18 @@ inline bool redirect(T &cli, const Request &req, Response &res, return ret; } +inline bool contains_header(const std::string &header, const std::string &name) { + if (header.length() >= name.length()) { + for (int i = 0; i < name.length(); ++i) { + if (std::tolower(header[i]) != std::tolower(name[i])) { + return false; + } + } + return true; + } + return false; +} + inline std::string params_to_query_str(const Params ¶ms) { std::string query; @@ -2914,8 +2933,6 @@ public: template bool parse(const char *buf, size_t n, T content_callback, U header_callback) { - static const std::regex re_content_type(R"(^Content-Type:\s*(.*?)\s*$)", - std::regex_constants::icase); static const std::regex re_content_disposition( "^Content-Disposition:\\s*form-data;\\s*name=\"(.*?)\"(?:;\\s*filename=" @@ -2961,8 +2978,11 @@ public: auto header = buf_.substr(0, pos); { std::smatch m; - if (std::regex_match(header, m, re_content_type)) { - file_.content_type = m[1]; + const std::string header_name = "content-type:"; + if (contains_header(header, header_name)) { + header.erase(header.begin(), header.begin() + header_name.size()); + trim(header); + file_.content_type = header; } else if (std::regex_match(header, m, re_content_disposition)) { file_.name = m[1]; file_.filename = m[2]; diff --git a/test/test.cc b/test/test.cc index 76678da..12c3fb3 100644 --- a/test/test.cc +++ b/test/test.cc @@ -43,6 +43,23 @@ TEST(StartupTest, WSAStartup) { ASSERT_EQ(0, ret); } #endif +TEST(TrimTests, TrimStringTests) { + { + std::string s = "abc"; + detail::trim(s); + EXPECT_EQ("abc", s); + } + { + std::string s = " abc "; + detail::trim(s); + EXPECT_EQ("abc", s); + } + { + std::string s = ""; + detail::trim(s); + EXPECT_TRUE( s.empty() ); + } +} TEST(SplitTest, ParseQueryString) { string s = "key1=val1&key2=val2&key3=val3"; @@ -1082,7 +1099,7 @@ protected: }) .Post("/multipart", [&](const Request &req, Response & /*res*/) { - EXPECT_EQ(5u, req.files.size()); + EXPECT_EQ(6u, req.files.size()); ASSERT_TRUE(!req.has_file("???")); ASSERT_TRUE(req.body.empty()); @@ -1111,6 +1128,13 @@ protected: EXPECT_EQ("application/octet-stream", file.content_type); EXPECT_EQ(0u, file.content.size()); } + + { + const auto &file = req.get_file_value("file4"); + EXPECT_TRUE(file.filename.empty()); + EXPECT_EQ(0u, file.content.size()); + EXPECT_EQ("application/json tmp-string", file.content_type); + } }) .Post("/empty", [&](const Request &req, Response &res) { @@ -1803,6 +1827,7 @@ TEST_F(ServerTest, MultipartFormData) { {"file1", "h\ne\n\nl\nl\no\n", "hello.txt", "text/plain"}, {"file2", "{\n \"world\", true\n}\n", "world.json", "application/json"}, {"file3", "", "", "application/octet-stream"}, + {"file4", "", "", " application/json tmp-string "} }; auto res = cli_.Post("/multipart", items);