From dce4e0c506d5014e84527cc9da64dd339f3e68a5 Mon Sep 17 00:00:00 2001 From: cpq Date: Wed, 16 Aug 2023 23:35:14 +0100 Subject: [PATCH] Fix #2322 - stricter Content-Length check, allow 1*DIGIT only --- mongoose.c | 34 +++++++++++++++++----------------- src/http.c | 34 +++++++++++++++++----------------- test/unit_test.c | 11 +++++++++-- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/mongoose.c b/mongoose.c index 698af52f518..0cf19d09887 100644 --- a/mongoose.c +++ b/mongoose.c @@ -1245,16 +1245,20 @@ struct mg_fs mg_fs_posix = {p_stat, p_list, p_open, p_close, p_read, bool mg_to_size_t(struct mg_str str, size_t *val); bool mg_to_size_t(struct mg_str str, size_t *val) { - uint64_t result = 0, max = 1844674407370955160 /* (UINT64_MAX-9)/10 */; - size_t i = 0; + size_t i = 0, max = (size_t) -1, max2 = max / 10, result = 0, ndigits = 0; while (i < str.len && (str.ptr[i] == ' ' || str.ptr[i] == '\t')) i++; if (i < str.len && str.ptr[i] == '-') return false; while (i < str.len && str.ptr[i] >= '0' && str.ptr[i] <= '9') { - if (result > max) return false; + size_t digit = (size_t) (str.ptr[i] - '0'); + if (result > max2) return false; // Overflow result *= 10; - result += (unsigned) (str.ptr[i] - '0'); - i++; + if (result > max - digit) return false; // Overflow + result += digit; + i++, ndigits++; } + while (i < str.len && (str.ptr[i] == ' ' || str.ptr[i] == '\t')) i++; + if (ndigits == 0) return false; // #2322: Content-Length = 1 * DIGIT + if (i != str.len) return false; // Ditto *val = (size_t) result; return true; } @@ -1746,20 +1750,16 @@ static struct mg_str guess_content_type(struct mg_str path, const char *extra) { static int getrange(struct mg_str *s, size_t *a, size_t *b) { size_t i, numparsed = 0; - // MG_INFO(("%.*s", (int) s->len, s->ptr)); for (i = 0; i + 6 < s->len; i++) { - if (memcmp(&s->ptr[i], "bytes=", 6) == 0) { - struct mg_str p = mg_str_n(s->ptr + i + 6, s->len - i - 6); - if (p.len > 0 && p.ptr[0] >= '0' && p.ptr[0] <= '9') numparsed++; - if (!mg_to_size_t(p, a)) return 0; - // MG_INFO(("PPP [%.*s] %d", (int) p.len, p.ptr, numparsed)); - while (p.len && p.ptr[0] >= '0' && p.ptr[0] <= '9') p.ptr++, p.len--; - if (p.len && p.ptr[0] == '-') p.ptr++, p.len--; - if (!mg_to_size_t(p, b)) return 0; - if (p.len > 0 && p.ptr[0] >= '0' && p.ptr[0] <= '9') numparsed++; - // MG_INFO(("PPP [%.*s] %d", (int) p.len, p.ptr, numparsed)); - break; + struct mg_str k, v = mg_str_n(s->ptr + i + 6, s->len - i - 6); + if (memcmp(&s->ptr[i], "bytes=", 6) != 0) continue; + if (mg_split(&v, &k, NULL, '-')) { + if (mg_to_size_t(k, a)) numparsed++; + if (v.len > 0 && mg_to_size_t(v, b)) numparsed++; + } else { + if (mg_to_size_t(v, a)) numparsed++; } + break; } return (int) numparsed; } diff --git a/src/http.c b/src/http.c index 7abe4702c46..b63752f5a40 100644 --- a/src/http.c +++ b/src/http.c @@ -13,16 +13,20 @@ bool mg_to_size_t(struct mg_str str, size_t *val); bool mg_to_size_t(struct mg_str str, size_t *val) { - uint64_t result = 0, max = 1844674407370955160 /* (UINT64_MAX-9)/10 */; - size_t i = 0; + size_t i = 0, max = (size_t) -1, max2 = max / 10, result = 0, ndigits = 0; while (i < str.len && (str.ptr[i] == ' ' || str.ptr[i] == '\t')) i++; if (i < str.len && str.ptr[i] == '-') return false; while (i < str.len && str.ptr[i] >= '0' && str.ptr[i] <= '9') { - if (result > max) return false; + size_t digit = (size_t) (str.ptr[i] - '0'); + if (result > max2) return false; // Overflow result *= 10; - result += (unsigned) (str.ptr[i] - '0'); - i++; + if (result > max - digit) return false; // Overflow + result += digit; + i++, ndigits++; } + while (i < str.len && (str.ptr[i] == ' ' || str.ptr[i] == '\t')) i++; + if (ndigits == 0) return false; // #2322: Content-Length = 1 * DIGIT + if (i != str.len) return false; // Ditto *val = (size_t) result; return true; } @@ -514,20 +518,16 @@ static struct mg_str guess_content_type(struct mg_str path, const char *extra) { static int getrange(struct mg_str *s, size_t *a, size_t *b) { size_t i, numparsed = 0; - // MG_INFO(("%.*s", (int) s->len, s->ptr)); for (i = 0; i + 6 < s->len; i++) { - if (memcmp(&s->ptr[i], "bytes=", 6) == 0) { - struct mg_str p = mg_str_n(s->ptr + i + 6, s->len - i - 6); - if (p.len > 0 && p.ptr[0] >= '0' && p.ptr[0] <= '9') numparsed++; - if (!mg_to_size_t(p, a)) return 0; - // MG_INFO(("PPP [%.*s] %d", (int) p.len, p.ptr, numparsed)); - while (p.len && p.ptr[0] >= '0' && p.ptr[0] <= '9') p.ptr++, p.len--; - if (p.len && p.ptr[0] == '-') p.ptr++, p.len--; - if (!mg_to_size_t(p, b)) return 0; - if (p.len > 0 && p.ptr[0] >= '0' && p.ptr[0] <= '9') numparsed++; - // MG_INFO(("PPP [%.*s] %d", (int) p.len, p.ptr, numparsed)); - break; + struct mg_str k, v = mg_str_n(s->ptr + i + 6, s->len - i - 6); + if (memcmp(&s->ptr[i], "bytes=", 6) != 0) continue; + if (mg_split(&v, &k, NULL, '-')) { + if (mg_to_size_t(k, a)) numparsed++; + if (v.len > 0 && mg_to_size_t(v, b)) numparsed++; + } else { + if (mg_to_size_t(v, a)) numparsed++; } + break; } return (int) numparsed; } diff --git a/test/unit_test.c b/test/unit_test.c index b0138920835..03b64c0d6e8 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -1579,6 +1579,7 @@ static void test_http_range(void) { fetch(&mgr, buf, url, "%s", "GET /range.txt HTTP/1.0\nRange: bytes=5-10\n\n"); ASSERT(mg_http_parse(buf, strlen(buf), &hm) > 0); + printf("%s", buf); ASSERT(mg_strcmp(hm.uri, mg_str("206")) == 0); ASSERT(mg_strcmp(hm.proto, mg_str("Partial Content")) == 0); ASSERT(mg_strcmp(hm.body, mg_str(" of co")) == 0); @@ -2184,16 +2185,22 @@ static void test_util(void) { { extern bool mg_to_size_t(struct mg_str, size_t *); - size_t val = 1; + size_t val, max = (size_t) -1; ASSERT(mg_to_size_t(mg_str("0"), &val) && val == 0); ASSERT(mg_to_size_t(mg_str("123"), &val) && val == 123); - ASSERT(mg_to_size_t(mg_str(""), &val) && val == 0); + ASSERT(mg_to_size_t(mg_str(" 123 \t"), &val) && val == 123); + ASSERT(mg_to_size_t(mg_str(""), &val) == false); + ASSERT(mg_to_size_t(mg_str(" 123x"), &val) == false); ASSERT(mg_to_size_t(mg_str("-"), &val) == false); + mg_snprintf(buf, sizeof(buf), "%lu", max); + ASSERT(mg_to_size_t(mg_str(buf), &val) && val == max); +#if 0 ASSERT(mg_to_size_t(mg_str("18446744073709551616"), &val) == false); // range +1 ASSERT(mg_to_size_t(mg_str("18446744073709551610"), &val) == false); // TODO(): ASSERT(mg_to_size_t(mg_str("18446744073709551609"), &val) && // val == 18446744073709551609U); // our max or SIZE_MAX +#endif } {