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.

diff --git a/lib/isc/time.c b/lib/isc/time.c
index 19f2ce33c6b448e98553c682b42c510897c2d5c2..76b74a81ab73e3da405cce27631df4cd3ec51b25 100644 (file)
--- 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",