Handle large numbers when parsing/printing a duration

The duration_fromtext() function is truncating large numbers
to 32 bits instead of capping or rejecting them, i.e. 64424509445,
which is 0xf00000005, gets parsed as 32-bit value 5 (0x00000005).

Fail parsing a duration if any of its components is bigger than
32 bits. Using those kind of big numbers has no practical use case
for a duration.

The cfg_obj_asduration() function can overflow the 32 bit
seconds variable when calculating the duration from its component
parts.

To avoid that, use 64-bit calculation and return UINT32_MAX if the
calculated value is bigger than UINT32_MAX. Again, a number this big
has no practical use case anyway.

The buffer for the generated duration string is limited to 64 bytes,
which, in theory, is smaller than the longest possible generated
duration string.

Use 80 bytes instead, calculated by the '7 x (10 + 1) + 3' formula,
where '7' is the count of the duration's parts (year, month, etc.), '10'
is their maximum length when printed as a decimal number, '1' is their
indicator character (Y, M, etc.), and 3 is two more indicators (P and T)
and the terminating NUL character.

(cherry picked from commit fddaebb285df4a7d00ff12f8a1bd9beef21a2a9d)

diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h
index b040eb84c73efd63627c9fcd980106f9fe8e0203..0bc40e06ba6c9ea33c06447b2f19c38bfa106436 100644 (file)
--- a/lib/isccfg/include/isccfg/grammar.h
+++ b/lib/isccfg/include/isccfg/grammar.h
@@ -86,7 +86,7 @@ typedef struct cfg_map            cfg_map_t;
 typedef struct cfg_rep     cfg_rep_t;
 typedef struct cfg_duration cfg_duration_t;
 
-#define CFG_DURATION_MAXLEN 64
+#define CFG_DURATION_MAXLEN 80
 
 /*
  * Function types for configuration object methods

diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c
index a1dc39cc1074a2a4acab19eb422caea53fd04d8c..117dda43ff3e1d7a91c3c9bf2eb31fb27dbc80f9 100644 (file)
--- a/lib/isccfg/parser.c
+++ b/lib/isccfg/parser.c
@@ -43,8 +43,10 @@
 /*! \file */
 
 #include <ctype.h>
+#include <errno.h>
 #include <inttypes.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdlib.h>
 
 #include <isc/buffer.h>
@@ -1143,32 +1145,38 @@ cfg_obj_isduration(const cfg_obj_t *obj) {
 
 uint32_t
 cfg_obj_asduration(const cfg_obj_t *obj) {
+       uint64_t seconds = 0;
+       cfg_duration_t duration;
+
        REQUIRE(obj != NULL && obj->type->rep == &cfg_rep_duration);
-       uint32_t duration = 0;
-       duration += obj->value.duration.parts[6];             /* Seconds */
-       duration += obj->value.duration.parts[5] * 60;        /* Minutes */
-       duration += obj->value.duration.parts[4] * 3600;      /* Hours */
-       duration += obj->value.duration.parts[3] * 86400;     /* Days */
-       duration += obj->value.duration.parts[2] * 86400 * 7; /* Weaks */
+
+       duration = obj->value.duration;
+
+       seconds += (uint64_t)duration.parts[6];             /* Seconds */
+       seconds += (uint64_t)duration.parts[5] * 60;        /* Minutes */
+       seconds += (uint64_t)duration.parts[4] * 3600;      /* Hours */
+       seconds += (uint64_t)duration.parts[3] * 86400;     /* Days */
+       seconds += (uint64_t)duration.parts[2] * 86400 * 7; /* Weeks */
        /*
         * The below additions are not entirely correct
-        * because days may very per month and per year.
+        * because days may vary per month and per year.
         */
-       duration += obj->value.duration.parts[1] * 86400 * 31;  /* Months */
-       duration += obj->value.duration.parts[0] * 86400 * 365; /* Years */
-       return (duration);
+       seconds += (uint64_t)duration.parts[1] * 86400 * 31;  /* Months */
+       seconds += (uint64_t)duration.parts[0] * 86400 * 365; /* Years */
+
+       return (seconds > UINT32_MAX ? UINT32_MAX : (uint32_t)seconds);
 }
 
 static isc_result_t
 duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
-       char buf[CFG_DURATION_MAXLEN];
+       char buf[CFG_DURATION_MAXLEN] = { 0 };
        char *P, *X, *T, *W, *str;
        bool not_weeks = false;
        int i;
+       long long int lli;
 
        /*
         * Copy the buffer as it may not be NULL terminated.
-        * Anyone having a duration longer than 63 characters is crazy.
         */
        if (source->length > sizeof(buf) - 1) {
                return (ISC_R_BADNUMBER);
@@ -1194,7 +1202,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
        /* Record years. */
        X = strpbrk(str, "Yy");
        if (X != NULL) {
-               duration->parts[0] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[0] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1207,7 +1220,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
         * part, or this M indicator is before the time indicator.
         */
        if (X != NULL && (T == NULL || (size_t)(X - P) < (size_t)(T - P))) {
-               duration->parts[1] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[1] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1215,7 +1233,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
        /* Record days. */
        X = strpbrk(str, "Dd");
        if (X != NULL) {
-               duration->parts[3] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[3] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1229,7 +1252,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
        /* Record hours. */
        X = strpbrk(str, "Hh");
        if (X != NULL && T != NULL) {
-               duration->parts[4] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[4] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1242,7 +1270,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
         * part and the M indicator is behind the time indicator.
         */
        if (X != NULL && T != NULL && (size_t)(X - P) > (size_t)(T - P)) {
-               duration->parts[5] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[5] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1250,7 +1283,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
        /* Record seconds. */
        X = strpbrk(str, "Ss");
        if (X != NULL && T != NULL) {
-               duration->parts[6] = atoi(str + 1);
+               errno = 0;
+               lli = strtoll(str + 1, NULL, 10);
+               if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                       return (ISC_R_BADNUMBER);
+               }
+               duration->parts[6] = (uint32_t)lli;
                str = X;
                not_weeks = true;
        }
@@ -1262,7 +1300,12 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) {
                        /* Mix of weeks and other indicators is not allowed */
                        return (ISC_R_BADNUMBER);
                } else {
-                       duration->parts[2] = atoi(str + 1);
+                       errno = 0;
+                       lli = strtoll(str + 1, NULL, 10);
+                       if (errno != 0 || lli < 0 || lli > UINT32_MAX) {
+                               return (ISC_R_BADNUMBER);
+                       }
+                       duration->parts[2] = (uint32_t)lli;
                        str = W;
                }
        }