libconfig: prevent overflow in config_parsebytesize

cyrusimap · Jun 30, 2023 · d45651f · d45651f
1 parent a4423be
commit d45651f
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 17 deletions.
diff --git a/cunit/libconfig.testc b/cunit/libconfig.testc
@@ -39,8 +39,9 @@
  */
 
 #include <limits.h>
-#include <unistd.h>
+#include <stdint.h>
 #include <sysexits.h>
+#include <unistd.h>
 
 #include "cunit/cyrunit.h"
 
@@ -765,6 +766,24 @@ static const struct bytesize_parse_data bytesize_parse_tests[] = {
     { "12  K",   0, 12LL * 1024 },
     { "12  KB",  0, 12LL * 1024 },
     { "12  KiB", 0, 12LL * 1024 },
+
+    /* overflow tests */
+    { "9223372036854775807 B", 0, INT64_MAX },
+    { "-9223372036854775808 B", 0, INT64_MIN },
+    { "9223372036854775808 B", -1, 0xdeadbeefLL },
+    { "-9223372036854775809 B", -1, 0xdeadbeefLL },
+    { "9007199254740991 K", 0, 1024LL * (INT64_MAX / 1024LL) },
+    { "-9007199254740992 K", 0, 1024LL * (INT64_MIN / 1024LL) },
+    { "9007199254740992 K", -1, 0xdeadbeefLL },
+    { "-9007199254740993 K", -1, 0xdeadbeefLL },
+    { "8796093022207 M", 0, 1048576LL * (INT64_MAX / 1048576LL) },
+    { "-8796093022208 M", 0, 1048576LL * (INT64_MIN / 1048576LL) },
+    { "8796093022208 M", -1, 0xdeadbeefLL },
+    { "-8796093022209 M", -1, 0xdeadbeefLL },
+    { "8589934591 G", 0, 1073741824LL * (INT64_MAX / 1073741824LL) },
+    { "-8589934592 G", 0, 1073741824LL * (INT64_MIN / 1073741824LL) },
+    { "8589934592 G", -1, 0xdeadbeefLL },
+    { "-8589934593 G", -1, 0xdeadbeefLL },
 };
 
 static void test_bytesize_parse(void)

diff --git a/lib/libconfig.c b/lib/libconfig.c
@@ -345,6 +345,7 @@ EXPORTED int config_parsebytesize(const char *str,
     int64_t bytesize;
     int i_allowed = 0, r = 0;
     char *copy = NULL, *p;
+    struct buf parse_err = BUF_INITIALIZER;
 
     assert(strchr("BKMG", defunit) != NULL); /* n.b. also permits \0 */
 
@@ -361,26 +362,16 @@ EXPORTED int config_parsebytesize(const char *str,
     errno = 0;
     bytesize = strtoll(copy, &p, 10);
     if (errno) {
-        xsyslog(LOG_ERR, "unable to parse byte size from string",
-                         "value=<%s>", str);
+        buf_setcstr(&parse_err, strerror(errno));
         errno = 0;
         r = -1;
         goto done;
     }
 
     /* better be some digits */
     if (p == copy) {
-        struct buf msg = BUF_INITIALIZER;
-
-        buf_appendcstr(&msg, "no digit ");
-        if (*p) {
-            buf_printf(&msg, "before '%c' ", *p);
-        }
-        buf_printf(&msg, "in '%s'", str);
-
-        syslog(LOG_DEBUG, "%s: %s", __func__, buf_cstring(&msg));
-        buf_free(&msg);
-
+        buf_setcstr(&parse_err, "no digit");
+        if (*p) buf_printf(&parse_err, " before '%c'", *p);
         r = -1;
         goto done;
     }
@@ -392,14 +383,29 @@ EXPORTED int config_parsebytesize(const char *str,
     switch (*p) {
     case 'g':
     case 'G':
+        if (bytesize > INT64_MAX / 1024 || bytesize < INT64_MIN / 1024) {
+            buf_printf(&parse_err, "would overflow at '%c'", *p);
+            r = -1;
+            goto done;
+        }
         bytesize *= 1024;
         /* fall through */
     case 'm':
     case 'M':
+        if (bytesize > INT64_MAX / 1024 || bytesize < INT64_MIN / 1024) {
+            buf_printf(&parse_err, "would overflow at '%c'", *p);
+            r = -1;
+            goto done;
+        }
         bytesize *= 1024;
         /* fall through */
     case 'k':
     case 'K':
+        if (bytesize > INT64_MAX / 1024 || bytesize < INT64_MIN / 1024) {
+            buf_printf(&parse_err, "would overflow at '%c'", *p);
+            r = -1;
+            goto done;
+        }
         bytesize *= 1024;
         i_allowed = 1;
         p++;
@@ -414,15 +420,22 @@ EXPORTED int config_parsebytesize(const char *str,
 
     /* we'd better be at end of string! */
     if (*p) {
-        syslog(LOG_DEBUG, "%s: bad unit '%c' in %s",
-                          __func__, *p, str);
+        buf_printf(&parse_err, "bad unit '%c'", *p);
         r = -1;
         goto done;
     }
 
 done:
-    if (!r && out_bytesize) *out_bytesize = bytesize;
+    if (r) {
+        xsyslog(LOG_ERR, "unable to parse bytesize from string",
+                         "value=<%s> parse_err=<%s>",
+                         str, buf_cstring_or_empty(&parse_err));
+    }
+    else if (out_bytesize) {
+        *out_bytesize = bytesize;
+    }
 
+    buf_free(&parse_err);
     free(copy);
     return r;
 }