Handle large numbers when parsing/printing a duration

The isccfg_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 isccfg_duration_toseconds() 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.

diff --git a/lib/isccfg/duration.c b/lib/isccfg/duration.c
index 305047e1af13edc85662732a99ab8413474da1d3..9ed9d6f65797ea9e879771550449008538a4576e 100644 (file)
--- a/lib/isccfg/duration.c
+++ b/lib/isccfg/duration.c
@@ -17,6 +17,7 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -39,14 +40,14 @@
 isc_result_t
 isccfg_duration_fromtext(isc_textregion_t *source,
                         isccfg_duration_t *duration) {
-       char buf[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);
@@ -74,7 +75,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
        /* 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;
        }
@@ -87,7 +93,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
         * 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;
        }
@@ -95,7 +106,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
        /* 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;
        }
@@ -109,7 +125,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
        /* 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;
        }
@@ -122,7 +143,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
         * 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;
        }
@@ -130,7 +156,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
        /* 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;
        }
@@ -142,7 +173,12 @@ isccfg_duration_fromtext(isc_textregion_t *source,
                        /* 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;
                }
        }
@@ -183,20 +219,21 @@ isccfg_parse_duration(isc_textregion_t *source, isccfg_duration_t *duration) {
 
 uint32_t
 isccfg_duration_toseconds(const isccfg_duration_t *duration) {
-       uint32_t seconds = 0;
+       uint64_t seconds = 0;
 
        REQUIRE(duration != NULL);
 
-       seconds += duration->parts[6];             /* Seconds */
-       seconds += duration->parts[5] * 60;        /* Minutes */
-       seconds += duration->parts[4] * 3600;      /* Hours */
-       seconds += duration->parts[3] * 86400;     /* Days */
-       seconds += duration->parts[2] * 86400 * 7; /* Weeks */
+       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.
         */
-       seconds += duration->parts[1] * 86400 * 31;  /* Months */
-       seconds += duration->parts[0] * 86400 * 365; /* Years */
-       return (seconds);
+       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);
 }

diff --git a/lib/isccfg/include/isccfg/duration.h b/lib/isccfg/include/isccfg/duration.h
index b64bea520f3b77860b7b4531e3341bdc7cb8a0ed..bd0c35bad8a0e1145e6ee9af30e102cadcd58ca0 100644 (file)
--- a/lib/isccfg/include/isccfg/duration.h
+++ b/lib/isccfg/include/isccfg/duration.h
@@ -24,7 +24,7 @@
 
 ISC_LANG_BEGINDECLS
 
-#define DURATION_MAXLEN 64
+#define CFG_DURATION_MAXLEN 80
 
 /*%
  * A configuration object to store ISO 8601 durations.
@@ -76,6 +76,10 @@ isccfg_duration_toseconds(const isccfg_duration_t *duration);
  * - Months will be treated as 31 days.
  * - Years will be treated as 365 days.
  *
+ * Notes:
+ *\li  If the duration in seconds is greater than UINT32_MAX, the return value
+ *     will be UINT32_MAX.
+ *
  * Returns:
  *\li  The duration in seconds.
  */

diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c
index 4ce73dd0b91a68af1adab5cd2445021f5e48cf33..e35b4e7f3bd28b9ae4dd80a976000065aeaa5d1f 100644 (file)
--- a/lib/isccfg/parser.c
+++ b/lib/isccfg/parser.c
@@ -1030,7 +1030,7 @@ numlen(uint32_t num) {
  */
 void
 cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) {
-       char buf[DURATION_MAXLEN];
+       char buf[CFG_DURATION_MAXLEN];
        char *str;
        const char *indicators = "YMWDHMS";
        int count, i;
@@ -1085,7 +1085,7 @@ cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) {
        if (T) {
                count++;
        }
-       INSIST(count < DURATION_MAXLEN);
+       INSIST(count < CFG_DURATION_MAXLEN);
 
        /* Now print the duration. */
        for (i = 0; i < 6; i++) {