From 0a9303bb07fd36c2dffdc4aeaeda006de0514609 Mon Sep 17 00:00:00 2001 From: cpq Date: Tue, 9 Feb 2021 21:16:33 +0000 Subject: [PATCH] Stricter mg_http_parse() --- mongoose.c | 8 ++++---- src/http.c | 10 ++++++---- test/unit_test.c | 15 +++++++++++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/mongoose.c b/mongoose.c index 38638504..3d3395bb 100644 --- a/mongoose.c +++ b/mongoose.c @@ -526,12 +526,12 @@ int mg_http_get_request_len(const unsigned char *buf, size_t buf_len) { return 0; } -static const char *skip(const char *s, const char *end, const char *delims, +static const char *skip(const char *s, const char *e, const char *d, struct mg_str *v) { v->ptr = s; - while (s < end && strchr(delims, *(unsigned char *) s) == NULL) s++; + while (s < e && *s != '\n' && strchr(d, *s) == NULL) s++; v->len = s - v->ptr; - while (s < end && strchr(delims, *(unsigned char *) s) != NULL) s++; + while (s < e && strchr(d, *s) != NULL) s++; return s; } @@ -579,7 +579,7 @@ int mg_http_parse(const char *s, size_t len, struct mg_http_message *hm) { s = skip(s, end, " ", &hm->method); s = skip(s, end, " ", &hm->uri); s = skip(s, end, "\r\n", &hm->proto); - if (hm->uri.ptr <= hm->method.ptr || hm->proto.ptr <= hm->uri.ptr) return -1; + if (hm->method.len == 0 || hm->uri.len == 0 || hm->proto.len == 0) return -1; // If URI contains '?' character, setup query string if ((qs = (const char *) memchr(hm->uri.ptr, '?', hm->uri.len)) != NULL) { diff --git a/src/http.c b/src/http.c index 49b118ae..0eecda58 100644 --- a/src/http.c +++ b/src/http.c @@ -106,12 +106,12 @@ int mg_http_get_request_len(const unsigned char *buf, size_t buf_len) { return 0; } -static const char *skip(const char *s, const char *end, const char *delims, +static const char *skip(const char *s, const char *e, const char *d, struct mg_str *v) { v->ptr = s; - while (s < end && strchr(delims, *(unsigned char *) s) == NULL) s++; + while (s < e && *s != '\n' && strchr(d, *s) == NULL) s++; v->len = s - v->ptr; - while (s < end && strchr(delims, *(unsigned char *) s) != NULL) s++; + while (s < e && strchr(d, *s) != NULL) s++; return s; } @@ -159,7 +159,9 @@ int mg_http_parse(const char *s, size_t len, struct mg_http_message *hm) { s = skip(s, end, " ", &hm->method); s = skip(s, end, " ", &hm->uri); s = skip(s, end, "\r\n", &hm->proto); - if (hm->uri.ptr <= hm->method.ptr || hm->proto.ptr <= hm->uri.ptr) return -1; + + // Sanity check + if (hm->method.len == 0 || hm->uri.len == 0 || hm->proto.len == 0) return -1; // If URI contains '?' character, setup query string if ((qs = (const char *) memchr(hm->uri.ptr, '?', hm->uri.len)) != NULL) { diff --git a/test/unit_test.c b/test/unit_test.c index c069a830..acc5dce5 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -848,6 +848,16 @@ static void test_http_parse(void) { ASSERT(mg_strcmp(hm.uri, mg_str("/foo")) == 0); ASSERT(mg_strcmp(hm.query, mg_str("bar=baz")) == 0); } + + { + struct mg_http_message hm; + const char *req = "a b c\n\n"; + ASSERT(mg_http_parse(req, strlen(req), &hm) == (int) strlen(req)); + req = "a b\nc\n\n"; + ASSERT(mg_http_parse(req, strlen(req), &hm) < 0); + req = "a\nb\nc\n\n"; + ASSERT(mg_http_parse(req, strlen(req), &hm) < 0); + } } static void test_http_range(void) { @@ -860,7 +870,8 @@ static void test_http_range(void) { mg_http_listen(&mgr, url, eh1, NULL); ASSERT(fetch(&mgr, buf, url, "GET /range.txt HTTP/1.0\n\n") == 200); - mg_http_parse(buf, strlen(buf), &hm); + ASSERT(mg_http_parse(buf, strlen(buf), &hm) > 0); + LOG(LL_INFO, ("----%d\n[%s]", (int) hm.body.len, buf)); ASSERT(hm.body.len == 312); // ASSERT(strlen(buf) == 312); @@ -1117,6 +1128,7 @@ static void test_util(void) { int main(void) { mg_log_set("3"); + test_http_parse(); test_util(); test_sntp(); test_dns(); @@ -1131,7 +1143,6 @@ int main(void) { test_http_get_var(); test_tls(); test_ws(); - test_http_parse(); test_http_server(); test_http_client(); test_http_no_content_length();