add alloc_array function

Adds a new memory allocation function for arrays which includes an overflow check on the size multiplication.
goatshriek · Oct 6, 2024 · 869bb49 · 869bb49
1 parent 1bd3333
commit 869bb49
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 8 deletions.
diff --git a/include/private/memory.h b/include/private/memory.h
@@ -25,5 +25,6 @@ void *alloc_mem( size_t size );
 void free_mem( const void *mem );
 size_t get_paged_size( size_t size );
 void *realloc_mem( const void *mem, size_t size );
+void *alloc_array( size_t item_count, size_t item_size );
 
 #endif /* __STUMPLESS_PRIVATE_MEMORY_H */
diff --git a/src/config/have_windows.c b/src/config/have_windows.c
@@ -65,7 +65,7 @@ windows_copy_cstring_to_lpwstr( LPCSTR str, int *copy_length ) {
     return NULL;
   }
 
-  str_copy = alloc_mem( needed_wchar_length * sizeof( WCHAR ) );
+  str_copy = alloc_array( needed_wchar_length, sizeof( WCHAR ) );
   if( !str_copy ) {
     return NULL;
   }
@@ -113,7 +113,7 @@ windows_copy_wstring_to_cstring( const wchar_t *str, int *copy_size ){
     return NULL;
   }
 
-  str_copy = alloc_mem( needed_size * sizeof( char ) );
+  str_copy = alloc_array( needed_size, sizeof( char ) );
   if( !str_copy ) {
     return NULL;
   }

diff --git a/src/config/wel_supported.c b/src/config/wel_supported.c
@@ -1898,7 +1898,7 @@ copy_param_value_to_lpwstr( const struct stumpless_param *param ) {
   }
 
   needed_wchar_count = ( ( size_t ) needed_wchar_length ) + 1;
-  str_copy = alloc_mem( needed_wchar_count * sizeof( WCHAR ) );
+  str_copy = alloc_array( needed_wchar_count, sizeof( WCHAR ) );
   if( !str_copy ) {
     goto fail;
   }
@@ -1952,12 +1952,12 @@ copy_wel_data( struct stumpless_entry *destination,
 
 
   if( source_data->insertion_count > 0 ) {
-    dest_data->insertion_params = alloc_mem( sizeof( struct stumpless_param * ) * source_data->insertion_count );
+    dest_data->insertion_params = alloc_array( source_data->insertion_count, sizeof( struct stumpless_param ) );
     if( !dest_data->insertion_params) {
       goto fail;
     }
 
-    dest_data->insertion_strings = alloc_mem( sizeof( LPCSTR ) * source_data->insertion_count );
+    dest_data->insertion_strings = alloc_array( source_data->insertion_count, sizeof( LPCSTR ) );
     if( !dest_data->insertion_strings) {
       goto fail_strings;
     }

diff --git a/src/element.c b/src/element.c
@@ -94,7 +94,7 @@ stumpless_copy_element( const struct stumpless_element *element ) {
     goto fail;
   }
 
-  copy->params = alloc_mem( element->param_count * sizeof( param_copy ) );
+  copy->params = alloc_array( element->param_count, sizeof( param_copy ) );
   if( !copy->params ) {
     goto fail_param_copy;
   }
@@ -212,7 +212,7 @@ stumpless_element_to_string( const struct stumpless_element *element ) {
     // acc total format size
     format_len = name_len;
 
-    params_format = alloc_mem(sizeof(char*) * param_count);
+    params_format = alloc_array( param_count, sizeof(char*) );
     for( i = 0; i < param_count; i++ ) {
       params_format[i] = stumpless_param_to_string(params[i]);
       // does not count '\0' on purpose

diff --git a/src/entry.c b/src/entry.c
@@ -157,7 +157,7 @@ stumpless_copy_entry( const struct stumpless_entry *entry ) {
     goto cleanup_and_fail;
   }
 
-  copy->elements = alloc_mem( entry->element_count * sizeof( element_copy ) );
+  copy->elements = alloc_array( entry->element_count, sizeof( element_copy ) );
   if( !copy->elements ) {
     goto fail_elements;
   }

diff --git a/src/memory.c b/src/memory.c
@@ -126,3 +126,12 @@ realloc_mem( const void *mem, size_t size ) {
 
   return new_mem;
 }
+
+void *
+alloc_array( size_t item_count, size_t item_size ) {
+  if (item_count && item_count >= (size_t)-1/item_size) {
+    raise_memory_allocation_failure();
+    return NULL;
+  }
+  return alloc_mem(item_count * item_size);
+}
diff --git a/test/function/entry.cpp b/test/function/entry.cpp
@@ -84,6 +84,8 @@ namespace {
         stumpless_add_element( basic_entry, element_2 );
 
         nil_entry = create_nil_entry();
+
+
       }
 
       virtual void
@@ -359,6 +361,7 @@ namespace {
     EXPECT_TRUE( set_malloc_result == malloc );
   }
 
+
   TEST_F( EntryTest, CopyReallocFailure ) {
     const struct stumpless_entry *result;
     void * (*set_realloc_result)(void *, size_t);
@@ -2891,4 +2894,26 @@ namespace {
     stumpless_unload_entry_only( NULL );
     EXPECT_ERROR_ID_EQ( STUMPLESS_ARGUMENT_EMPTY );
   }
+
+  TEST( CopyEntry, MallocFailureOnSizeOverflow ) {
+    const struct stumpless_entry *result;
+    struct stumpless_entry *large_entry = NULL;
+	size_t large_entry_ec;
+
+    large_entry = stumpless_new_entry_str( STUMPLESS_FACILITY_USER,
+			STUMPLESS_SEVERITY_INFO,
+			"large-app-name",
+			"large-msgid",
+			"large message" );
+	large_entry_ec = large_entry->element_count;
+	large_entry->element_count = SIZE_MAX;
+
+	result = stumpless_copy_entry( large_entry );
+	EXPECT_NULL( result );
+	EXPECT_ERROR_ID_EQ( STUMPLESS_MEMORY_ALLOCATION_FAILURE );
+
+    large_entry->element_count = large_entry_ec;
+    stumpless_destroy_entry_and_contents(large_entry);
+  }
+
 }