Skip to content

Commit 852a374

Browse files
omjegoyhiroseFefer-Ivan
authored
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 <[email protected]> Co-authored-by: yhirose <[email protected]> Co-authored-by: Ivan Fefer <[email protected]>
1 parent 3b5bab3 commit 852a374

File tree

2 files changed

+50
-5
lines changed

2 files changed

+50
-5
lines changed

httplib.h

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,13 @@ inline std::pair<int, int> trim(const char *b, const char *e, int left,
14521452
return std::make_pair(left, right);
14531453
}
14541454

1455+
inline void trim(std::string &s) {
1456+
auto is_not_space = [](int ch) { return !std::isspace(ch); };
1457+
s.erase(s.begin(), std::find_if(s.begin(), s.end(), is_not_space));
1458+
s.erase(std::find_if(s.rbegin(), s.rend(), is_not_space).base(), s.end());
1459+
}
1460+
1461+
14551462
template <class Fn> void split(const char *b, const char *e, char d, Fn fn) {
14561463
int i = 0;
14571464
int beg = 0;
@@ -2828,6 +2835,18 @@ inline bool redirect(T &cli, const Request &req, Response &res,
28282835
return ret;
28292836
}
28302837

2838+
inline bool contains_header(const std::string &header, const std::string &name) {
2839+
if (header.length() >= name.length()) {
2840+
for (int i = 0; i < name.length(); ++i) {
2841+
if (std::tolower(header[i]) != std::tolower(name[i])) {
2842+
return false;
2843+
}
2844+
}
2845+
return true;
2846+
}
2847+
return false;
2848+
}
2849+
28312850
inline std::string params_to_query_str(const Params &params) {
28322851
std::string query;
28332852

@@ -2914,8 +2933,6 @@ class MultipartFormDataParser {
29142933

29152934
template <typename T, typename U>
29162935
bool parse(const char *buf, size_t n, T content_callback, U header_callback) {
2917-
static const std::regex re_content_type(R"(^Content-Type:\s*(.*?)\s*$)",
2918-
std::regex_constants::icase);
29192936

29202937
static const std::regex re_content_disposition(
29212938
"^Content-Disposition:\\s*form-data;\\s*name=\"(.*?)\"(?:;\\s*filename="
@@ -2961,8 +2978,11 @@ class MultipartFormDataParser {
29612978
auto header = buf_.substr(0, pos);
29622979
{
29632980
std::smatch m;
2964-
if (std::regex_match(header, m, re_content_type)) {
2965-
file_.content_type = m[1];
2981+
const std::string header_name = "content-type:";
2982+
if (contains_header(header, header_name)) {
2983+
header.erase(header.begin(), header.begin() + header_name.size());
2984+
trim(header);
2985+
file_.content_type = header;
29662986
} else if (std::regex_match(header, m, re_content_disposition)) {
29672987
file_.name = m[1];
29682988
file_.filename = m[2];

test/test.cc

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@ TEST(StartupTest, WSAStartup) {
4343
ASSERT_EQ(0, ret);
4444
}
4545
#endif
46+
TEST(TrimTests, TrimStringTests) {
47+
{
48+
std::string s = "abc";
49+
detail::trim(s);
50+
EXPECT_EQ("abc", s);
51+
}
52+
{
53+
std::string s = " abc ";
54+
detail::trim(s);
55+
EXPECT_EQ("abc", s);
56+
}
57+
{
58+
std::string s = "";
59+
detail::trim(s);
60+
EXPECT_TRUE( s.empty() );
61+
}
62+
}
4663

4764
TEST(SplitTest, ParseQueryString) {
4865
string s = "key1=val1&key2=val2&key3=val3";
@@ -1082,7 +1099,7 @@ class ServerTest : public ::testing::Test {
10821099
})
10831100
.Post("/multipart",
10841101
[&](const Request &req, Response & /*res*/) {
1085-
EXPECT_EQ(5u, req.files.size());
1102+
EXPECT_EQ(6u, req.files.size());
10861103
ASSERT_TRUE(!req.has_file("???"));
10871104
ASSERT_TRUE(req.body.empty());
10881105

@@ -1111,6 +1128,13 @@ class ServerTest : public ::testing::Test {
11111128
EXPECT_EQ("application/octet-stream", file.content_type);
11121129
EXPECT_EQ(0u, file.content.size());
11131130
}
1131+
1132+
{
1133+
const auto &file = req.get_file_value("file4");
1134+
EXPECT_TRUE(file.filename.empty());
1135+
EXPECT_EQ(0u, file.content.size());
1136+
EXPECT_EQ("application/json tmp-string", file.content_type);
1137+
}
11141138
})
11151139
.Post("/empty",
11161140
[&](const Request &req, Response &res) {
@@ -1803,6 +1827,7 @@ TEST_F(ServerTest, MultipartFormData) {
18031827
{"file1", "h\ne\n\nl\nl\no\n", "hello.txt", "text/plain"},
18041828
{"file2", "{\n \"world\", true\n}\n", "world.json", "application/json"},
18051829
{"file3", "", "", "application/octet-stream"},
1830+
{"file4", "", "", " application/json tmp-string "}
18061831
};
18071832

18081833
auto res = cli_.Post("/multipart", items);

0 commit comments

Comments
 (0)