add some more overflow treatment and make sure to set errno properly

Sat, 21 Dec 2024 21:03:28 +0100

author
Mike Becker <universe@uap-core.de>
date
Sat, 21 Dec 2024 21:03:28 +0100
changeset 1040
1ecf4dbbc60c
parent 1039
ec62453fc8a6
child 1041
508dc8b32a17

add some more overflow treatment and make sure to set errno properly

resolves #469

src/allocator.c file | annotate | diff | comparison | revisions
src/array_list.c file | annotate | diff | comparison | revisions
src/buffer.c file | annotate | diff | comparison | revisions
src/cx/string.h file | annotate | diff | comparison | revisions
src/hash_map.c file | annotate | diff | comparison | revisions
src/json.c file | annotate | diff | comparison | revisions
src/mempool.c file | annotate | diff | comparison | revisions
src/string.c file | annotate | diff | comparison | revisions
src/tree.c file | annotate | diff | comparison | revisions
tests/test_allocator.c file | annotate | diff | comparison | revisions
--- a/src/allocator.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/allocator.c	Sat Dec 21 21:03:28 2024 +0100
@@ -99,7 +99,7 @@
 ) {
     size_t n;
     if (cx_szmul(nmemb, size, &n)) {
-        errno = ENOMEM;
+        errno = EOVERFLOW;
         return 1;
     } else {
         void *nmem = realloc(*mem, n);
@@ -137,7 +137,7 @@
 ) {
     size_t n;
     if (cx_szmul(nmemb, size, &n)) {
-        errno = ENOMEM;
+        errno = EOVERFLOW;
         return NULL;
     } else {
         return allocator->cl->realloc(allocator->data, mem, n);
--- a/src/array_list.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/array_list.c	Sat Dec 21 21:03:28 2024 +0100
@@ -40,7 +40,12 @@
         size_t elem_size,
         cx_attr_unused CxArrayReallocator *alloc
 ) {
-    return realloc(array, capacity * elem_size);
+    size_t n;
+    if (cx_szmul(capacity, elem_size, &n)) {
+        errno = EOVERFLOW;
+        return NULL;
+    }
+    return realloc(array, n);
 }
 
 CxArrayReallocator cx_array_default_reallocator_impl = {
@@ -57,18 +62,25 @@
         size_t elem_size,
         cx_attr_unused CxArrayReallocator *alloc
 ) {
+    // check for overflow
+    size_t n;
+    if (cx_szmul(capacity, elem_size, &n)) {
+        errno = EOVERFLOW;
+        return NULL;
+    }
+
     // retrieve the pointer to the actual allocator
     const CxAllocator *al = alloc->ptr1;
 
     // check if the array is still located on the stack
     void *newmem;
     if (array == alloc->ptr2) {
-        newmem = cxMalloc(al, capacity * elem_size);
+        newmem = cxMalloc(al, n);
         if (newmem != NULL && array != NULL) {
-            memcpy(newmem, array, capacity * elem_size);
+            memcpy(newmem, array, n);
         }
     } else {
-        newmem = cxRealloc(al, array, capacity * elem_size);
+        newmem = cxRealloc(al, array, n);
     }
     return newmem;
 }
@@ -89,6 +101,18 @@
 
 // LOW LEVEL ARRAY LIST FUNCTIONS
 
+static size_t cx_array_align_capacity(
+        size_t cap,
+        size_t alignment,
+        size_t max
+) {
+    if (cap > max - alignment) {
+        return cap;
+    } else {
+        return cap - (cap % alignment) + alignment;
+    }
+}
+
 int cx_array_reserve(
         void **array,
         void *size,
@@ -136,19 +160,19 @@
     // assert that the array is allocated when it has capacity
     assert(*array != NULL || oldcap == 0);
 
-    // determine new capacity
-    size_t newcap = oldsize + elem_count;
-
     // check for overflow
-    if (newcap > max_size) {
+    if (elem_count > max_size - oldsize) {
         errno = EOVERFLOW;
         return 1;
     }
 
+    // determine new capacity
+    size_t newcap = oldsize + elem_count;
+
     // reallocate if possible
     if (newcap > oldcap) {
         // calculate new capacity (next number divisible by 16)
-        newcap = newcap - (newcap % 16) + 16;
+        newcap = cx_array_align_capacity(newcap, 16, max_size);
 
         // perform reallocation
         void *newmem = reallocator->realloc(
@@ -229,16 +253,16 @@
     // assert that the array is allocated when it has capacity
     assert(*target != NULL || oldcap == 0);
 
+    // check for overflow
+    if (index > max_size || elem_count > max_size - index) {
+        errno = EOVERFLOW;
+        return 1;
+    }
+
     // check if resize is required
     size_t minsize = index + elem_count;
     size_t newsize = oldsize < minsize ? minsize : oldsize;
 
-    // check for overflow
-    if (newsize > max_size) {
-        errno = EOVERFLOW;
-        return 1;
-    }
-
     // reallocate if possible
     size_t newcap = oldcap;
     if (newsize > oldcap) {
@@ -249,7 +273,7 @@
                          && srcaddr < targetaddr + oldcap * elem_size;
 
         // calculate new capacity (next number divisible by 16)
-        newcap = newsize - (newsize % 16) + 16;
+        newcap = cx_array_align_capacity(newsize, 16, max_size);
         assert(newcap > newsize);
 
         // perform reallocation
@@ -274,6 +298,7 @@
     start += index * elem_size;
 
     // copy elements and set new size
+    // note: no overflow check here, b/c we cannot get here w/o allocation
     memmove(start, src, elem_count * elem_size);
 
     // if any of size or capacity changed, store them back
@@ -321,13 +346,19 @@
     // corner case
     if (elem_count == 0) return 0;
 
+    // overflow check
+    if (elem_count > SIZE_MAX - *size) {
+        errno = EOVERFLOW;
+        return 1;
+    }
+
     // store some counts
     size_t old_size = *size;
     size_t needed_capacity = old_size + elem_count;
 
     // if we need more than we have, try a reallocation
     if (needed_capacity > *capacity) {
-        size_t new_capacity = needed_capacity - (needed_capacity % 16) + 16;
+        size_t new_capacity = cx_array_align_capacity(needed_capacity, 16, SIZE_MAX);
         void *new_mem = reallocator->realloc(
                 *target, new_capacity, elem_size, reallocator
         );
--- a/src/buffer.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/buffer.c	Sat Dec 21 21:03:28 2024 +0100
@@ -30,6 +30,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 
 static int buffer_copy_on_write(CxBuffer* buffer) {
     if (0 == (buffer->flags & CX_BUFFER_COPY_ON_WRITE)) return 0;
@@ -136,6 +137,7 @@
     npos += offset;
 
     if ((offset > 0 && npos < opos) || (offset < 0 && npos > opos)) {
+        errno = EOVERFLOW;
         return -1;
     }
 
@@ -247,6 +249,7 @@
     size_t len;
     size_t nitems_out = nitems;
     if (cx_szmul(size, nitems, &len)) {
+        errno = EOVERFLOW;
         return 0;
     }
     size_t required = buffer->pos + len;
@@ -285,6 +288,7 @@
     if (perform_flush) {
         size_t flush_max;
         if (cx_szmul(buffer->flush_blkmax, buffer->flush_blksize, &flush_max)) {
+            errno = EOVERFLOW;
             return 0;
         }
         size_t flush_pos = buffer->flush_func == NULL || buffer->flush_target == NULL
@@ -385,6 +389,7 @@
 ) {
     size_t len;
     if (cx_szmul(size, nitems, &len)) {
+        errno = EOVERFLOW;
         return 0;
     }
     if (buffer->pos + len > buffer->size) {
@@ -436,6 +441,10 @@
         CxBuffer *buffer,
         size_t shift
 ) {
+    if (buffer->size > SIZE_MAX - shift) {
+        errno = EOVERFLOW;
+        return -1;
+    }
     size_t req_capacity = buffer->size + shift;
     size_t movebytes;
 
--- a/src/cx/string.h	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/cx/string.h	Sat Dec 21 21:03:28 2024 +0100
@@ -290,6 +290,8 @@
 
 /**
  * Returns the accumulated length of all specified strings.
+ * 
+ * If this sum overflows, errno is set to EOVERFLOW.
  *
  * \attention if the count argument is larger than the number of the
  * specified strings, the behavior is undefined.
--- a/src/hash_map.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/hash_map.c	Sat Dec 21 21:03:28 2024 +0100
@@ -30,6 +30,7 @@
 
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 
 struct cx_hash_map_element_s {
     /** A pointer to the next element in the current bucket. */
@@ -451,6 +452,10 @@
     if (map->collection.size > ((hash_map->bucket_count * 3) >> 2)) {
 
         size_t new_bucket_count = (map->collection.size * 5) >> 1;
+        if (new_bucket_count < hash_map->bucket_count) {
+            errno = EOVERFLOW;
+            return 1;
+        }
         struct cx_hash_map_element_s **new_buckets = cxCalloc(
                 map->collection.allocator,
                 new_bucket_count, sizeof(struct cx_hash_map_element_s *)
--- a/src/json.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/json.c	Sat Dec 21 21:03:28 2024 +0100
@@ -26,14 +26,14 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include "cx/json.h"
+
 #include <string.h>
 #include <ctype.h>
 #include <assert.h>
 #include <stdio.h>
 #include <errno.h>
 
-#include "cx/json.h"
-
 /*
  * RFC 8259
  * https://tools.ietf.org/html/rfc8259
--- a/src/mempool.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/mempool.c	Sat Dec 21 21:03:28 2024 +0100
@@ -29,6 +29,7 @@
 #include "cx/mempool.h"
 
 #include <string.h>
+#include <errno.h>
 
 struct cx_mempool_memory_s {
     /** The destructor. */
@@ -45,7 +46,13 @@
 
     if (pool->size >= pool->capacity) {
         size_t newcap = pool->capacity - (pool->capacity % 16) + 16;
-        struct cx_mempool_memory_s **newdata = realloc(pool->data, newcap*sizeof(struct cx_mempool_memory_s*));
+        size_t newmsize;
+        if (pool->capacity > newcap || cx_szmul(newcap,
+                sizeof(struct cx_mempool_memory_s*), &newmsize)) {
+            errno = EOVERFLOW;
+            return NULL;
+        }
+        struct cx_mempool_memory_s **newdata = realloc(pool->data, newmsize);
         if (newdata == NULL) {
             return NULL;
         }
@@ -72,6 +79,7 @@
 ) {
     size_t msz;
     if (cx_szmul(nelem, elsize, &msz)) {
+        errno = EOVERFLOW;
         return NULL;
     }
     void *ptr = cx_mempool_malloc(p, msz);
@@ -204,6 +212,7 @@
 ) {
     size_t poolsize;
     if (cx_szmul(capacity, sizeof(struct cx_mempool_memory_s*), &poolsize)) {
+        errno = EOVERFLOW;
         return NULL;
     }
 
--- a/src/string.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/string.c	Sat Dec 21 21:03:28 2024 +0100
@@ -92,6 +92,7 @@
     size_t size = 0;
     for (size_t i = 0; i < count; i++) {
         cxstring str = va_arg(ap, cxstring);
+        if (size > SIZE_MAX - str.length) errno = EOVERFLOW;
         size += str.length;
     }
     va_end(ap);
@@ -122,14 +123,25 @@
     va_start(ap, count);
 
     // get all args and overall length
+    bool overflow = false;
     size_t slen = str.length;
     for (size_t i = 0; i < count; i++) {
         cxstring s = va_arg (ap, cxstring);
         strings[i] = s;
+        if (slen > SIZE_MAX - str.length) overflow = true;
         slen += s.length;
     }
     va_end(ap);
 
+    // abort in case of overflow
+    if (overflow) {
+        errno = EOVERFLOW;
+        if (strings != strings_stack) {
+            free(strings);
+        }
+        return (cxmutstr) { NULL, 0 };
+    }
+
     // reallocate or create new string
     char *newstr;
     if (str.ptr == NULL) {
@@ -138,7 +150,9 @@
         newstr = cxRealloc(alloc, str.ptr, slen + 1);
     }
     if (newstr == NULL) {
-        free(strings);
+        if (strings != strings_stack) {
+            free(strings);
+        }
         return (cxmutstr) {NULL, 0};
     }
     str.ptr = newstr;
--- a/src/tree.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/src/tree.c	Sat Dec 21 21:03:28 2024 +0100
@@ -403,7 +403,7 @@
     return iter->node;
 }
 
-__attribute__((__nonnull__))
+cx_attr_nonnull
 static void cx_tree_visitor_enqueue_siblings(
         struct cx_tree_visitor_s *iter, void *node, ptrdiff_t loc_next) {
     node = tree_next(node);
--- a/tests/test_allocator.c	Fri Dec 20 21:25:33 2024 +0100
+++ b/tests/test_allocator.c	Sat Dec 21 21:03:28 2024 +0100
@@ -90,7 +90,7 @@
     memcpy(test, "Test", 5);
     CX_TEST_DO {
         void *fail = cxReallocArray(cxDefaultAllocator, test, SIZE_MAX/2, 4);
-        CX_TEST_ASSERT(errno == ENOMEM);
+        CX_TEST_ASSERT(errno == EOVERFLOW);
         CX_TEST_ASSERT(fail == NULL);
         CX_TEST_ASSERT(0 == strcmp(test, "Test"));
     }
@@ -148,7 +148,7 @@
     CX_TEST_DO {
         int ret = cxReallocateArray(cxDefaultAllocator, &test, SIZE_MAX/2, 4);
         CX_TEST_ASSERT(ret != 0);
-        CX_TEST_ASSERT(errno == ENOMEM);
+        CX_TEST_ASSERT(errno == EOVERFLOW);
         CX_TEST_ASSERT(test != NULL);
         CX_TEST_ASSERT(0 == strcmp(test, "Test"));
     }
@@ -173,7 +173,7 @@
     CX_TEST_DO {
         int ret = cx_reallocatearray(&test, SIZE_MAX/2, 4);
         CX_TEST_ASSERT(ret != 0);
-        CX_TEST_ASSERT(errno == ENOMEM);
+        CX_TEST_ASSERT(errno == EOVERFLOW);
         CX_TEST_ASSERT(test != NULL);
         CX_TEST_ASSERT(0 == strcmp(test, "Test"));
     }

mercurial