From 8c05f12bc8abd27808abc7e21fffef88d2922e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 20 Oct 2021 11:22:52 +0200 Subject: [PATCH] Fix isc_time_add() overflow The isc_time_add() could overflow when t.seconds + i.seconds == UINT_MAX and t.nanoseconds + i.nanoseconds >= NS_PER_S. Fix the overflow in isc_time_add(), and simplify the ISC_R_RANGE checks both in isc_time_add() and isc_time_subtract() functions. --- lib/isc/time.c | 64 +++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/isc/time.c b/lib/isc/time.c index 19f2ce33c6..76b74a81ab 100644 --- a/lib/isc/time.c +++ b/lib/isc/time.c @@ -228,25 +228,22 @@ isc_time_compare(const isc_time_t *t1, const isc_time_t *t2) { isc_result_t isc_time_add(const isc_time_t *t, const isc_interval_t *i, isc_time_t *result) { REQUIRE(t != NULL && i != NULL && result != NULL); - INSIST(t->nanoseconds < NS_PER_S && i->nanoseconds < NS_PER_S); + REQUIRE(t->nanoseconds < NS_PER_S && i->nanoseconds < NS_PER_S); - /* - * Ensure the resulting seconds value fits in the size of an - * unsigned int. (It is written this way as a slight optimization; - * note that even if both values == INT_MAX, then when added - * and getting another 1 added below the result is UINT_MAX.) - */ - if ((t->seconds > INT_MAX || i->seconds > INT_MAX) && - ((long long)t->seconds + i->seconds > UINT_MAX)) - { + /* Seconds */ + if (t->seconds > UINT_MAX - i->seconds) { return (ISC_R_RANGE); } - result->seconds = t->seconds + i->seconds; + + /* Nanoseconds */ result->nanoseconds = t->nanoseconds + i->nanoseconds; if (result->nanoseconds >= NS_PER_S) { - result->seconds++; + if (result->seconds == UINT_MAX) { + return (ISC_R_RANGE); + } result->nanoseconds -= NS_PER_S; + result->seconds++; } return (ISC_R_SUCCESS); @@ -256,22 +253,24 @@ isc_result_t isc_time_subtract(const isc_time_t *t, const isc_interval_t *i, isc_time_t *result) { REQUIRE(t != NULL && i != NULL && result != NULL); - INSIST(t->nanoseconds < NS_PER_S && i->nanoseconds < NS_PER_S); + REQUIRE(t->nanoseconds < NS_PER_S && i->nanoseconds < NS_PER_S); - if ((unsigned int)t->seconds < i->seconds || - ((unsigned int)t->seconds == i->seconds && - t->nanoseconds < i->nanoseconds)) - { + /* Seconds */ + if (t->seconds < i->seconds) { return (ISC_R_RANGE); } - result->seconds = t->seconds - i->seconds; + + /* Nanoseconds */ if (t->nanoseconds >= i->nanoseconds) { result->nanoseconds = t->nanoseconds - i->nanoseconds; } else { - result->nanoseconds = NS_PER_S - i->nanoseconds + - t->nanoseconds; + if (result->seconds == 0) { + return (ISC_R_RANGE); + } result->seconds--; + result->nanoseconds = NS_PER_S + t->nanoseconds - + i->nanoseconds; } return (ISC_R_SUCCESS); @@ -318,20 +317,20 @@ isc_time_secondsastimet(const isc_time_t *t, time_t *secondsp) { /* * Ensure that the number of seconds represented by t->seconds - * can be represented by a time_t. Since t->seconds is an unsigned - * int and since time_t is mostly opaque, this is trickier than - * it seems. (This standardized opaqueness of time_t is *very* - * frustrating; time_t is not even limited to being an integral - * type.) + * can be represented by a time_t. Since t->seconds is an + * unsigned int and since time_t is mostly opaque, this is + * trickier than it seems. (This standardized opaqueness of + * time_t is *very* frustrating; time_t is not even limited to + * being an integral type.) * * The mission, then, is to avoid generating any kind of warning - * about "signed versus unsigned" while trying to determine if the - * the unsigned int t->seconds is out range for tv_sec, which is - * pretty much only true if time_t is a signed integer of the same - * size as the return value of isc_time_seconds. + * about "signed versus unsigned" while trying to determine if + * the unsigned int t->seconds is out range for tv_sec, + * which is pretty much only true if time_t is a signed integer + * of the same size as the return value of isc_time_seconds. * - * If the paradox in the if clause below is true, t->seconds is out - * of range for time_t. + * If the paradox in the if clause below is true, t->seconds is + * out of range for time_t. */ seconds = (time_t)t->seconds; @@ -390,7 +389,8 @@ isc_time_formathttptimestamp(const isc_time_t *t, char *buf, unsigned int len) { REQUIRE(len > 0); /* - * 5 spaces, 1 comma, 3 GMT, 2 %d, 4 %Y, 8 %H:%M:%S, 3+ %a, 3+ %b (29+) + * 5 spaces, 1 comma, 3 GMT, 2 %d, 4 %Y, 8 %H:%M:%S, 3+ %a, 3+ + * %b (29+) */ now = (time_t)t->seconds; flen = strftime(buf, len, "%a, %d %b %Y %H:%M:%S GMT",