commit f51418aabcc2af47f61a97f818b013ade6e45208 Author: teor <teor2345@gmail.com> Date: Sun Oct 12 20:50:10 2014 +1100 Avoid overflow in format_time_interval, create unit tests Fix an instance of integer overflow in format_time_interval() when taking the absolute value of the supplied signed interval value. Fixes bug 13393. Create unit tests for format_time_interval(). --- .../bug13393-format-time-interval-overflow-test | 6 + src/common/util.c | 6 +- src/test/test_util.c | 423 ++++++++++++++++++++ 3 files changed, 434 insertions(+), 1 deletion(-) diff --git a/changes/bug13393-format-time-interval-overflow-test b/changes/bug13393-format-time-interval-overflow-test new file mode 100644 index 0000000..cc15572 --- /dev/null +++ b/changes/bug13393-format-time-interval-overflow-test @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Fix an instance of integer overflow in format_time_interval(). + Fixes bug 13393. + + o Minor features (test): + - Create unit tests for format_time_interval(). With bug 13393. diff --git a/src/common/util.c b/src/common/util.c index f4d293c..0ea7095 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1670,7 +1670,11 @@ format_time_interval(char *out, size_t out_len, long interval) { /* We only report seconds if there's no hours. */ long sec = 0, min = 0, hour = 0, day = 0; - if (interval < 0) + + /* -LONG_MIN is LONG_MAX + 1, which causes signed overflow */ + if (interval < -LONG_MAX) + interval = LONG_MAX; + else if (interval < 0) interval = -interval; if (interval >= 86400) { diff --git a/src/test/test_util.c b/src/test/test_util.c index c67aa71..3fed1f7 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -22,6 +22,7 @@ #include <tchar.h> #endif #include <math.h> +#include <ctype.h> /* XXXX this is a minimal wrapper to make the unit tests compile with the * changed tor_timegm interface. */ @@ -2068,6 +2069,427 @@ test_util_sscanf(void *arg) ; } +#define tt_char_op(a,op,b) tt_assert_op_type(a,op,b,char,"%c") +#define tt_ci_char_op(a,op,b) tt_char_op(tolower(a),op,tolower(b)) + +static void +test_util_format_time_interval(void *arg) +{ + /* use the same sized buffer and integers as tor uses */ +#define DBUF_SIZE 64 + char dbuf[DBUF_SIZE]; +#define T_ "%ld" + long sec, min, hour, day; + + /* we don't care about the exact spelling of the + * second(s), minute(s), hour(s), day(s) labels */ +#define LABEL_SIZE 21 +#define L_ "%20s" + char label_s[LABEL_SIZE]; + char label_m[LABEL_SIZE]; + char label_h[LABEL_SIZE]; + char label_d[LABEL_SIZE]; + +#define TL_ T_ " " L_ + + int r; + + (void)arg; + + /* In these tests, we're not picky about + * spelling or abbreviations */ + + /* seconds: 0, 1, 9, 10, 59 */ + + /* ignore exact spelling of "second(s)"*/ + format_time_interval(dbuf, sizeof(dbuf), 0); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 0); + + format_time_interval(dbuf, sizeof(dbuf), 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 1); + + format_time_interval(dbuf, sizeof(dbuf), 10); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 10); + + format_time_interval(dbuf, sizeof(dbuf), 59); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 59); + + /* negative seconds are reported as their absolute value */ + + format_time_interval(dbuf, sizeof(dbuf), -4); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 4); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + + format_time_interval(dbuf, sizeof(dbuf), -32); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &sec, label_s); + tt_int_op(r,==, 2); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(sec,==, 32); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + + /* minutes: 1:00, 1:01, 1:59, 2:00, 2:01, 59:59 */ + + /* ignore trailing "0 second(s)", if present */ + format_time_interval(dbuf, sizeof(dbuf), 60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &min, label_m); + tt_int_op(r,==, 2); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(min,==, 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + + /* ignore exact spelling of "minute(s)," and "second(s)" */ + format_time_interval(dbuf, sizeof(dbuf), 60 + 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 1); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 1); + tt_ci_char_op(label_s[0],==, 's'); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + + format_time_interval(dbuf, sizeof(dbuf), 60*2 - 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 1); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 59); + tt_ci_char_op(label_s[0],==, 's'); + + /* ignore trailing "0 second(s)", if present */ + format_time_interval(dbuf, sizeof(dbuf), 60*2); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &min, label_m); + tt_int_op(r,==, 2); + tt_int_op(min,==, 2); + tt_ci_char_op(label_m[0],==, 'm'); + + /* ignore exact spelling of "minute(s)," and "second(s)" */ + format_time_interval(dbuf, sizeof(dbuf), 60*2 + 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 2); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 1); + tt_ci_char_op(label_s[0],==, 's'); + + format_time_interval(dbuf, sizeof(dbuf), 60*60 - 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 59); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 59); + tt_ci_char_op(label_s[0],==, 's'); + + /* negative minutes are reported as their absolute value */ + + /* ignore trailing "0 second(s)", if present */ + format_time_interval(dbuf, sizeof(dbuf), -3*60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &min, label_m); + tt_int_op(r,==, 2); + tt_int_op(min,==, 3); + tt_ci_char_op(label_m[0],==, 'm'); + + /* ignore exact spelling of "minute(s)," and "second(s)" */ + format_time_interval(dbuf, sizeof(dbuf), -96); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 1); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 36); + tt_ci_char_op(label_s[0],==, 's'); + + format_time_interval(dbuf, sizeof(dbuf), -2815); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &min, label_m, &sec, label_s); + tt_int_op(r,==, 4); + tt_int_op(min,==, 46); + tt_ci_char_op(label_m[0],==, 'm'); + tt_int_op(sec,==, 55); + tt_ci_char_op(label_s[0],==, 's'); + + /* hours: 1:00, 1:00:01, 1:01, 23:59, 23:59:59 */ + /* always ignore trailing seconds, if present */ + + /* ignore trailing "0 minute(s)" etc., if present */ + format_time_interval(dbuf, sizeof(dbuf), 60*60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &hour, label_h); + tt_int_op(r,==, 2); + tt_int_op(hour,==, 1); + tt_ci_char_op(label_h[0],==, 'h'); + + format_time_interval(dbuf, sizeof(dbuf), 60*60 + 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &hour, label_h); + tt_int_op(r,==, 2); + tt_int_op(hour,==, 1); + tt_ci_char_op(label_h[0],==, 'h'); + + /* ignore exact spelling of "hour(s)," etc. */ + format_time_interval(dbuf, sizeof(dbuf), 60*60 + 60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &hour, label_h, &min, label_m); + tt_int_op(r,==, 4); + tt_int_op(hour,==, 1); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 1); + tt_ci_char_op(label_m[0],==, 'm'); + + format_time_interval(dbuf, sizeof(dbuf), 24*60*60 - 60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &hour, label_h, &min, label_m); + tt_int_op(r,==, 4); + tt_int_op(hour,==, 23); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 59); + tt_ci_char_op(label_m[0],==, 'm'); + + format_time_interval(dbuf, sizeof(dbuf), 24*60*60 - 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &hour, label_h, &min, label_m); + tt_int_op(r,==, 4); + tt_int_op(hour,==, 23); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 59); + tt_ci_char_op(label_m[0],==, 'm'); + + /* negative hours are reported as their absolute value */ + + /* ignore exact spelling of "hour(s)," etc., if present */ + format_time_interval(dbuf, sizeof(dbuf), -2*60*60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &hour, label_h); + tt_int_op(r,==, 2); + tt_int_op(hour,==, 2); + tt_ci_char_op(label_h[0],==, 'h'); + + format_time_interval(dbuf, sizeof(dbuf), -75804); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &hour, label_h, &min, label_m); + tt_int_op(r,==, 4); + tt_int_op(hour,==, 21); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 3); + tt_ci_char_op(label_m[0],==, 'm'); + + /* days: 1:00, 1:00:00:01, 1:00:01, 1:01 */ + /* always ignore trailing seconds, if present */ + + /* ignore trailing "0 hours(s)" etc., if present */ + format_time_interval(dbuf, sizeof(dbuf), 24*60*60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &day, label_d); + tt_int_op(r,==, 2); + tt_int_op(day,==, 1); + tt_ci_char_op(label_d[0],==, 'd'); + + format_time_interval(dbuf, sizeof(dbuf), 24*60*60 + 1); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_, &day, label_d); + tt_int_op(r,==, 2); + tt_int_op(day,==, 1); + tt_ci_char_op(label_d[0],==, 'd'); + + /* ignore exact spelling of "days(s)," etc. */ + format_time_interval(dbuf, sizeof(dbuf), 24*60*60 + 60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + if (r == -1) { + /* ignore 0 hours(s), if present */ + r = tor_sscanf(dbuf, TL_ " " TL_, + &day, label_d, &min, label_m); + } + tt_assert(r == 4 || r == 6); + tt_int_op(day,==, 1); + tt_ci_char_op(label_d[0],==, 'd'); + if (r == 6) { + tt_int_op(hour,==, 0); + tt_ci_char_op(label_h[0],==, 'h'); + } + tt_int_op(min,==, 1); + tt_ci_char_op(label_m[0],==, 'm'); + + /* ignore trailing "0 minutes(s)" etc., if present */ + format_time_interval(dbuf, sizeof(dbuf), 24*60*60 + 60*60); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_, + &day, label_d, &hour, label_h); + tt_int_op(r,==, 4); + tt_int_op(day,==, 1); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 1); + tt_ci_char_op(label_h[0],==, 'h'); + + /* negative days are reported as their absolute value */ + + format_time_interval(dbuf, sizeof(dbuf), -21936184); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 253); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 21); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 23); + tt_ci_char_op(label_m[0],==, 'm'); + + /* periods > 1 year are reported in days (warn?) */ + + /* ignore exact spelling of "days(s)," etc., if present */ + format_time_interval(dbuf, sizeof(dbuf), 758635154); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 8780); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 11); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 59); + tt_ci_char_op(label_m[0],==, 'm'); + + /* negative periods > 1 year are reported in days (warn?) */ + + format_time_interval(dbuf, sizeof(dbuf), -1427014922); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 16516); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 9); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 2); + tt_ci_char_op(label_m[0],==, 'm'); + +#if SIZEOF_LONG == 4 || SIZEOF_LONG == 8 + + /* We can try INT32_MIN/MAX */ + /* Always ignore second(s) */ + + /* INT32_MAX */ + format_time_interval(dbuf, sizeof(dbuf), 2147483647); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 24855); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 3); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 14); + tt_ci_char_op(label_m[0],==, 'm'); + /* and 7 seconds - ignored */ + + /* INT32_MIN: check that we get the absolute value of interval, + * which doesn't actually fit in int32_t. + * We expect INT32_MAX or INT32_MAX + 1 with 64 bit longs */ + format_time_interval(dbuf, sizeof(dbuf), -2147483647L - 1L); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 24855); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 3); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 14); + tt_ci_char_op(label_m[0],==, 'm'); + /* and 7 or 8 seconds - ignored */ + +#endif + +#if SIZEOF_LONG == 8 + + /* We can try INT64_MIN/MAX */ + /* Always ignore second(s) */ + + /* INT64_MAX */ + format_time_interval(dbuf, sizeof(dbuf), 9223372036854775807L); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 106751991167300L); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 15); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 30); + tt_ci_char_op(label_m[0],==, 'm'); + /* and 7 seconds - ignored */ + + /* INT64_MIN: check that we get the absolute value of interval, + * which doesn't actually fit in int64_t. + * We expect INT64_MAX */ + format_time_interval(dbuf, sizeof(dbuf), + -9223372036854775807L - 1L); + tt_int_op(strnlen(dbuf, DBUF_SIZE),<=, DBUF_SIZE - 1); + r = tor_sscanf(dbuf, TL_ " " TL_ " " TL_, + &day, label_d, &hour, label_h, &min, label_m); + tt_int_op(r,==, 6); + tt_int_op(day,==, 106751991167300L); + tt_ci_char_op(label_d[0],==, 'd'); + tt_int_op(hour,==, 15); + tt_ci_char_op(label_h[0],==, 'h'); + tt_int_op(min,==, 30); + tt_ci_char_op(label_m[0],==, 'm'); + /* and 7 or 8 seconds - ignored */ + +#endif + + +done: + ; +} + +#undef tt_char_op +#undef tt_ci_char_op +#undef DBUF_SIZE +#undef T_ +#undef LABEL_SIZE +#undef L_ +#undef TL_ + static void test_util_path_is_relative(void *arg) { @@ -4146,6 +4568,7 @@ struct testcase_t util_tests[] = { UTIL_LEGACY(mmap), UTIL_LEGACY(threads), UTIL_LEGACY(sscanf), + UTIL_LEGACY(format_time_interval), UTIL_LEGACY(path_is_relative), UTIL_LEGACY(strtok), UTIL_LEGACY(di_ops),