You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2019/03/28 22:00:15 UTC
[trafficserver] branch master updated: Don't allow header values to
overflow into negative values
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 4621f36 Don't allow header values to overflow into negative values
4621f36 is described below
commit 4621f36e587a51cad4a6b015205a6ddd07aa60c2
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Tue Mar 26 10:32:10 2019 +0100
Don't allow header values to overflow into negative values
For example, CC: max-age=2147483648 will turn into a negative values
and hence, can not be served out of cache. Since this parser deals with
proper negative value, it seems reasonable to clamp this to INT_MAX.
This fixes several issue from the cache-tests harness.
---
proxy/hdrs/MIME.cc | 121 ++++++++++++++++++++++++++++++++----------------
proxy/hdrs/MIME.h | 12 ++---
proxy/hdrs/test_mime.cc | 41 ++++++++++++++++
3 files changed, 128 insertions(+), 46 deletions(-)
diff --git a/proxy/hdrs/MIME.cc b/proxy/hdrs/MIME.cc
index 8d22324..c23e264 100644
--- a/proxy/hdrs/MIME.cc
+++ b/proxy/hdrs/MIME.cc
@@ -3061,12 +3061,18 @@ mime_parse_int(const char *buf, const char *end)
return 0;
}
- if (is_digit(*buf)) // fast case
- {
+ if (is_digit(*buf)) { // fast case
num = *buf++ - '0';
while ((buf != end) && is_digit(*buf)) {
- num = (num * 10) + (*buf++ - '0');
+ if (num != INT_MAX) {
+ int new_num = (num * 10) + (*buf++ - '0');
+
+ num = (new_num < num ? INT_MAX : new_num); // Check for overflow
+ } else {
+ ++buf; // Skip the remaining (valid) digits since we reached MAX/MIN_INT
+ }
}
+
return num;
} else {
num = 0;
@@ -3083,7 +3089,13 @@ mime_parse_int(const char *buf, const char *end)
// NOTE: we first compute the value as negative then correct the
// sign back to positive. This enables us to correctly parse MININT.
while ((buf != end) && is_digit(*buf)) {
- num = (num * 10) - (*buf++ - '0');
+ if (num != INT_MIN) {
+ int new_num = (num * 10) - (*buf++ - '0');
+
+ num = (new_num > num ? INT_MIN : new_num); // Check for overflow, so to speak, see above re: negative
+ } else {
+ ++buf; // Skip the remaining (valid) digits since we reached MAX/MIN_INT
+ }
}
if (!negative) {
@@ -3418,7 +3430,7 @@ mime_parse_date(const char *buf, const char *end)
return t;
}
-int
+bool
mime_parse_day(const char *&buf, const char *end, int *day)
{
const char *e;
@@ -3434,14 +3446,14 @@ mime_parse_day(const char *&buf, const char *end, int *day)
*day = day_names_dfa->match({buf, size_t(e - buf)});
if (*day < 0) {
- return 0;
+ return false;
} else {
buf = e;
- return 1;
+ return true;
}
}
-int
+bool
mime_parse_month(const char *&buf, const char *end, int *month)
{
const char *e;
@@ -3457,20 +3469,20 @@ mime_parse_month(const char *&buf, const char *end, int *month)
*month = month_names_dfa->match({buf, size_t(e - buf)});
if (*month < 0) {
- return 0;
+ return false;
} else {
buf = e;
- return 1;
+ return true;
}
}
-int
+bool
mime_parse_mday(const char *&buf, const char *end, int *mday)
{
return mime_parse_integer(buf, end, mday);
}
-int
+bool
mime_parse_year(const char *&buf, const char *end, int *year)
{
int val;
@@ -3480,7 +3492,7 @@ mime_parse_year(const char *&buf, const char *end, int *year)
}
if ((buf == end) || (*buf == '\0')) {
- return 0;
+ return false;
}
val = 0;
@@ -3497,59 +3509,88 @@ mime_parse_year(const char *&buf, const char *end, int *year)
*year = val;
- return 1;
+ return true;
}
-int
+bool
mime_parse_time(const char *&buf, const char *end, int *hour, int *min, int *sec)
{
if (!mime_parse_integer(buf, end, hour)) {
- return 0;
+ return false;
}
if (!mime_parse_integer(buf, end, min)) {
- return 0;
+ return false;
}
if (!mime_parse_integer(buf, end, sec)) {
- return 0;
+ return false;
}
- return 1;
+ return true;
}
-// TODO: Do we really need mime_parse_int() and mime_parse_integer() ? I know
-// they have slightly different prototypes, but still...
-int
+// This behaves slightly different than mime_parse_int(), int that we actually
+// return a "bool" for success / failure on "reasonable" parsing. This kinda
+// dumb, because we have two interfaces, where one does not move along the
+// buf pointer, but this one does (and the ones using this function do).
+bool
mime_parse_integer(const char *&buf, const char *end, int *integer)
{
- int val;
- bool negative;
-
- negative = false;
-
while ((buf != end) && *buf && !is_digit(*buf) && (*buf != '-')) {
buf += 1;
}
if ((buf == end) || (*buf == '\0')) {
- return 0;
+ return false;
}
- if (*buf == '-') {
- negative = true;
- buf += 1;
- }
+ int32_t num;
+ bool negative;
- val = 0;
- while ((buf != end) && is_digit(*buf)) {
- val = (val * 10) + (*buf++ - '0');
- }
+ // This code is copied verbatim from mime_parse_int ... Sigh. Maybe amc is right, and
+ // we really need to clean this up. But, as such, we should redo all these interfaces,
+ // and that's a big undertaking (and we'd want to move these strings all to string_view's).
+ if (is_digit(*buf)) { // fast case
+ num = *buf++ - '0';
+ while ((buf != end) && is_digit(*buf)) {
+ if (num != INT_MAX) {
+ int new_num = (num * 10) + (*buf++ - '0');
- if (negative) {
- *integer = -val;
+ num = (new_num < num ? INT_MAX : new_num); // Check for overflow
+ } else {
+ ++buf; // Skip the remaining (valid) digits since we reached MAX/MIN_INT
+ }
+ }
} else {
- *integer = val;
+ num = 0;
+ negative = false;
+
+ while ((buf != end) && ParseRules::is_space(*buf)) {
+ buf += 1;
+ }
+
+ if ((buf != end) && (*buf == '-')) {
+ negative = true;
+ buf += 1;
+ }
+ // NOTE: we first compute the value as negative then correct the
+ // sign back to positive. This enables us to correctly parse MININT.
+ while ((buf != end) && is_digit(*buf)) {
+ if (num != INT_MIN) {
+ int new_num = (num * 10) - (*buf++ - '0');
+
+ num = (new_num > num ? INT_MIN : new_num); // Check for overflow, so to speak, see above re: negative
+ } else {
+ ++buf; // Skip the remaining (valid) digits since we reached MAX/MIN_INT
+ }
+ }
+
+ if (!negative) {
+ num = -num;
+ }
}
- return 1;
+ *integer = num;
+
+ return true;
}
/***********************************************************************
diff --git a/proxy/hdrs/MIME.h b/proxy/hdrs/MIME.h
index de5ea81..311b5cc 100644
--- a/proxy/hdrs/MIME.h
+++ b/proxy/hdrs/MIME.h
@@ -784,12 +784,12 @@ uint32_t mime_parse_uint(const char *buf, const char *end = nullptr);
int64_t mime_parse_int64(const char *buf, const char *end = nullptr);
int mime_parse_rfc822_date_fastcase(const char *buf, int length, struct tm *tp);
time_t mime_parse_date(const char *buf, const char *end = nullptr);
-int mime_parse_day(const char *&buf, const char *end, int *day);
-int mime_parse_month(const char *&buf, const char *end, int *month);
-int mime_parse_mday(const char *&buf, const char *end, int *mday);
-int mime_parse_year(const char *&buf, const char *end, int *year);
-int mime_parse_time(const char *&buf, const char *end, int *hour, int *min, int *sec);
-int mime_parse_integer(const char *&buf, const char *end, int *integer);
+bool mime_parse_day(const char *&buf, const char *end, int *day);
+bool mime_parse_month(const char *&buf, const char *end, int *month);
+bool mime_parse_mday(const char *&buf, const char *end, int *mday);
+bool mime_parse_year(const char *&buf, const char *end, int *year);
+bool mime_parse_time(const char *&buf, const char *end, int *hour, int *min, int *sec);
+bool mime_parse_integer(const char *&buf, const char *end, int *integer);
/***********************************************************************
* *
diff --git a/proxy/hdrs/test_mime.cc b/proxy/hdrs/test_mime.cc
index b5055f6..eba287d 100644
--- a/proxy/hdrs/test_mime.cc
+++ b/proxy/hdrs/test_mime.cc
@@ -52,6 +52,47 @@ REGRESSION_TEST(MIME)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatu
hdr.destroy();
}
+REGRESSION_TEST(MIME_PARSERS)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus)
+{
+ const char *end;
+ int value;
+ TestBox box(t, pstatus);
+ box = REGRESSION_TEST_PASSED;
+
+ std::vector<std::pair<const char *, int>> tests = {{"0", 0},
+ {"1234", 1234},
+ {"-1234", -1234},
+ {"2147483647", 2147483647},
+ {"-2147483648", 2147483648},
+ {"2147483648", INT_MAX},
+ {"-2147483649", INT_MIN},
+ {"2147483647", INT_MAX},
+ {"-2147483648", INT_MIN},
+ {"999999999999", INT_MAX},
+ {"-999999999999", INT_MIN}};
+
+ for (const auto &it : tests) {
+ auto [buf, val] = it;
+
+ end = buf + strlen(buf);
+ box.check(mime_parse_int(buf, end) == val, "Failed mime_parse_int");
+ box.check(mime_parse_integer(buf, end, &value), "Failed mime_parse_integer call");
+ box.check(value == val, "Failed mime_parse_integer value");
+ }
+
+ // Also check the date parser, which relies heavily on the mime_parse_integer() function
+ const char *date1 = "Sun, 05 Dec 1999 08:49:37 GMT";
+ const char *date2 = "Sunday, 05-Dec-1999 08:49:37 GMT";
+
+ int d1 = mime_parse_date(date1, date1 + strlen(date1));
+ int d2 = mime_parse_date(date2, date2 + strlen(date2));
+
+ box.check(d1 == d2, "Failed mime_parse_date");
+
+ printf("Date1: %d\n", d1);
+ printf("Date2: %d\n", d2);
+}
+
int
main(int argc, const char **argv)
{