From 869bb49c1b130f6148f1fb8e123772427789f597 Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Mon, 7 Oct 2024 05:26:06 +0530 Subject: [PATCH] add alloc_array function Adds a new memory allocation function for arrays which includes an overflow check on the size multiplication. --- include/private/memory.h | 1 + src/config/have_windows.c | 4 ++-- src/config/wel_supported.c | 6 +++--- src/element.c | 4 ++-- src/entry.c | 2 +- src/memory.c | 9 +++++++++ test/function/entry.cpp | 25 +++++++++++++++++++++++++ 7 files changed, 43 insertions(+), 8 deletions(-) diff --git a/include/private/memory.h b/include/private/memory.h index 82ed283c4..5bdfc8cbc 100644 --- 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 index 9acaf06f4..6ce57b4af 100644 --- 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 index 690c082a3..122f29660 100644 --- 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 index 4124074d0..0abb513eb 100644 --- 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 index db63bc6c1..2ec6c9478 100644 --- 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 index 817237c6b..ec334e845 100644 --- 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 index 03d7d3820..6c1370929 100644 --- 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); + } + }