alloc: Guard against multiplication overflow

src/CMakeLists.txt

@@ -53,6 +53,7 @@ unittest(parser_test "")
 unittest(resizable_test "resizable.h")
 unittest(hashmap_test "hashmap.h")
 unittest(strings_test "")
+unittest(alloc_test "")
 
 function(functionaltest name)
     add_test(NAME "functionaltest_${name}" COMMAND functional-test-runner ${CMAKE_SOURCE_DIR}/src/functional-tests/${name}.at)

src/alloc.c

@@ -2,6 +2,8 @@
 #include <stddef.h>
 #include <stdlib.h>
 
+#include <stdint.h>
+
 #include "apfl.h"
 #include "alloc.h"
 
@@ -80,3 +82,35 @@ apfl_allocator_default(void)
         ? apfl_allocator_wrap_verifying(&libc_allocator)
         : libc_allocator;
 }
+
+#ifdef __GNUC__
+# define MULTIPLY_CHECK_OVERFLOW(res, a, b) __builtin_mul_overflow((a), (b), &(res))
+#else
+# define MULTIPLY_CHECK_OVERFLOW(res, a, b) \
+    (                                       \
+        (b) != 0 && (a) > SIZE_MAX / (b)    \
+            ? true                          \
+            : ((res = (a) * (b)), false)    \
+    )
+#endif
+
+void *
+apfl_alloc_realloc_array(
+    struct apfl_allocator allocator,
+    void *oldptr,
+    size_t elem_size,
+    size_t oldlen,
+    size_t newlen
+) {
+    size_t oldsize;
+    size_t newsize;
+
+    if (MULTIPLY_CHECK_OVERFLOW(oldsize, elem_size, oldlen)) {
+        return NULL;
+    }
+    if (MULTIPLY_CHECK_OVERFLOW(newsize, elem_size, newlen)) {
+        return NULL;
+    }
+
+    return REALLOC_BYTES(allocator, oldptr, oldsize, newsize);
+}

src/alloc.h

@@ -7,6 +7,8 @@ extern "C" {
 
 #include <stddef.h>
 
+#include "apfl.h"
+
 #define ALLOCATOR_CALL(a, oldptr, oldsize, newsize) \
     ((a).alloc((a).opaque, (oldptr), (oldsize), (newsize)))
 
@@ -14,9 +16,13 @@ extern "C" {
 #define REALLOC_BYTES(a, ptr, old, new) ALLOCATOR_CALL(a, ptr, old, new)
 #define FREE_BYTES(a, ptr, n) ALLOCATOR_CALL(a, ptr, n, 0)
 
-#define REALLOC_LIST(a, ptr, old, new) REALLOC_BYTES(a, ptr, sizeof(*ptr) * (old), sizeof(*ptr) * (new))
-#define ALLOC_LIST(a, T, len) (T *)ALLOC_BYTES(a, sizeof(T) * (len))
-#define FREE_LIST(a, ptr, len) FREE_BYTES(a, ptr, sizeof(*ptr) * (len))
+#define REALLOC_UNTYPED_ARRAY(a, ptr, elemsize, old, new) apfl_alloc_realloc_array(a, (ptr), (elemsize), (old), (new))
+#define ALLOC_UNTYPED_ARRAY(a, elemsize, len) REALLOC_UNTYPED_ARRAY((a), NULL, (elemsize), 0, (len))
+#define FREE_UNTYPED_ARRAY(a, ptr, elemsize, len) REALLOC_UNTYPED_ARRAY((a), (ptr), (elemsize), (len), 0)
+
+#define REALLOC_LIST(a, ptr, old, new) REALLOC_UNTYPED_ARRAY((a), (ptr), sizeof(*(ptr)), (old), (new))
+#define ALLOC_LIST(a, T, len) (T *)ALLOC_UNTYPED_ARRAY((a), sizeof(T), (len))
+#define FREE_LIST(a, ptr, len) FREE_UNTYPED_ARRAY((a), (ptr), sizeof(*ptr), (len))
 
 #define ALLOC_OBJ(a, T) ALLOC_LIST(a, T, 1)
 #define FREE_OBJ(a, ptr) FREE_LIST(a, ptr, 1)
@@ -96,6 +102,8 @@ do {                                                             \
         (var) = NULL;                     \
     } while(0)
 
+void *apfl_alloc_realloc_array(struct apfl_allocator, void *oldptr, size_t elem_size, size_t oldlen, size_t newlen);
+
 #ifdef __cplusplus
 }
 #endif

src/alloc_test.c

@@ -0,0 +1,25 @@
+#include <stdint.h>
+
+#include "test.h"
+
+#include "alloc.h"
+
+#define ELEMSIZE 256
+
+typedef char elemtype[ELEMSIZE];
+
+TEST(overflow, t) {
+    t->allocation_autofail = false;
+
+    struct apfl_allocator allocator = test_allocator(t);
+    size_t maxelems = SIZE_MAX / sizeof(elemtype);
+
+    elemtype *mem = ALLOC_LIST(allocator, elemtype, maxelems + 2);
+    if (mem != NULL) {
+        test_fatalf(t, "Could unexpectedly allocate more than maxelems elements.");
+    }
+}
+
+TESTS_BEGIN
+    ADDTEST(overflow),
+TESTS_END

src/hashmap.c

@@ -176,11 +176,12 @@ prepare_set_in_bucket(
 
         void *newmem;
 
-        newmem = REALLOC_BYTES(
+        newmem = REALLOC_UNTYPED_ARRAY(
             map->allocator,
             bucket->keys,
-            bucket->keys_cap * keysize,
-            new_cap * keysize
+            keysize,
+            bucket->keys_cap,
+            new_cap
         );
         if (newmem == NULL) {
             return false;
@@ -188,11 +189,12 @@ prepare_set_in_bucket(
         bucket->keys = newmem;
         bucket->keys_cap = new_cap;
 
-        newmem = REALLOC_BYTES(
+        newmem = REALLOC_UNTYPED_ARRAY(
             map->allocator,
             bucket->values,
-            bucket->values_cap * valsize,
-            new_cap * valsize
+            valsize,
+            bucket->values_cap,
+            new_cap
         );
         if (newmem == NULL) {
             return false;
@@ -363,8 +365,8 @@ destroy_bucket(const struct apfl_hashmap map, struct apfl_hashmap_bucket *bucket
         destroy_key(map, KVADDR(bucket->keys, map.keysize, i));
         destroy_value(map, KVADDR(bucket->values, map.valsize, i));
     }
-    FREE_BYTES(map.allocator, bucket->keys, bucket->keys_cap * map.keysize);
-    FREE_BYTES(map.allocator, bucket->values, bucket->values_cap * map.valsize);
+    FREE_UNTYPED_ARRAY(map.allocator, bucket->keys, map.keysize, bucket->keys_cap);
+    FREE_UNTYPED_ARRAY(map.allocator, bucket->values, map.valsize, bucket->values_cap);
     bucket->len = 0;
     bucket->keys_cap = 0;
     bucket->values_cap = 0;
@@ -426,12 +428,12 @@ apfl_hashmap_copy(struct apfl_hashmap *dst, struct apfl_hashmap src)
             continue;
         }
 
-        dstbucket->keys = ALLOC_BYTES(src.allocator, keysize * len);
-        dstbucket->values = ALLOC_BYTES(src.allocator, valsize * len);
+        dstbucket->keys = ALLOC_UNTYPED_ARRAY(src.allocator, keysize, len);
+        dstbucket->values = ALLOC_UNTYPED_ARRAY(src.allocator, valsize, len);
         if (dstbucket->keys == NULL || dstbucket->values == NULL) {
-            FREE_BYTES(src.allocator, dstbucket->keys, keysize * len);
+            FREE_UNTYPED_ARRAY(src.allocator, dstbucket->keys, keysize, len);
             dstbucket->keys = NULL;
-            FREE_BYTES(src.allocator, dstbucket->values, valsize * len);
+            FREE_UNTYPED_ARRAY(src.allocator, dstbucket->values, valsize, len);
             dstbucket->values = NULL;
 
             goto fail;

src/resizable.c

@@ -55,7 +55,7 @@ apfl_resizable_ensure_cap(
     // TODO: We currently simply grow the memory to have space for exactly
     //       want_cap elements. It would probably be smarter to grow the memory
     //       a bit larger to reduce calls to realloc.
-    void *newmem = REALLOC_BYTES(allocator, *mem, *cap * elem_size, want_cap * elem_size);
+    void *newmem = REALLOC_UNTYPED_ARRAY(allocator, *mem, elem_size, *cap, want_cap);
     if (newmem == NULL) {
         return false;
     }

src/test.h

@@ -21,6 +21,7 @@ struct testctx_struct {
     const char *prefix;
     struct apfl_allocator allocator_unverified;
     bool ok;
+    bool allocation_autofail;
 };
 
 typedef struct testctx_struct *testctx;
@@ -85,7 +86,7 @@ test_must_allocator_cb(void *opaque, void *oldptr, size_t oldsize, size_t newsiz
         return NULL;
     } else {
         void *mem = realloc(oldptr, newsize);
-        if (mem == NULL) {
+        if (t->allocation_autofail && mem == NULL) {
             test_fatalf(t, "Failed to allocate memory");
         }
         return mem;
@@ -104,7 +105,10 @@ bool
 test_run_test(struct testdef test)
 {
     char* prefix = malloc(TESTPREFIXSIZE);
+
     testctx t = malloc(sizeof(struct testctx_struct));
+    t->allocation_autofail = true;
+
     jmp_buf* here = malloc(sizeof(jmp_buf));
     if(prefix == NULL || t == NULL || here == NULL) {
         fprintf(stderr, "Could not execute test '%s': could not allocate memory.\n", test.name);