From c598946c010d269c7c214f5812d2ab97f7693966 Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Sun, 6 Oct 2024 00:46:20 +0530 Subject: [PATCH 1/5] Added alloc_array --- include/private/memory.h | 1 + src/memory.c | 9 +++++++++ 2 files changed, 10 insertions(+) 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/memory.c b/src/memory.c index 817237c6b..3dfcd6613 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); +} From 360b569c9aa0b7959eeee4fe49399bd7ab4a2b38 Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Sun, 6 Oct 2024 00:59:53 +0530 Subject: [PATCH 2/5] Replaced alloc_mem with alloc_array where appropriate --- src/config/have_windows.c | 4 ++-- src/config/wel_supported.c | 6 +++--- src/element.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) 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 From ff97dcfc9d417aea2f99fc433c3969c6d9d1e70b Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Sun, 6 Oct 2024 01:16:55 +0530 Subject: [PATCH 3/5] Added EntryTest CopyMallocFailureOnSizeOverflow --- src/entry.c | 2 +- test/function/entry.cpp | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) 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/test/function/entry.cpp b/test/function/entry.cpp index 03d7d3820..1450824e0 100644 --- a/test/function/entry.cpp +++ b/test/function/entry.cpp @@ -65,6 +65,7 @@ namespace { const char *param_1_1_value = "basic-value"; struct stumpless_param *param_1_1 = NULL; struct stumpless_entry *nil_entry = NULL; + struct stumpless_entry *large_entry = NULL; virtual void SetUp( void ) { @@ -84,6 +85,13 @@ namespace { stumpless_add_element( basic_entry, element_2 ); nil_entry = create_nil_entry(); + + large_entry = stumpless_new_entry_str( STUMPLESS_FACILITY_USER, + STUMPLESS_SEVERITY_INFO, + basic_app_name, + basic_msgid, + basic_message ); + large_entry->element_count = SIZE_MAX; } virtual void @@ -359,6 +367,14 @@ namespace { EXPECT_TRUE( set_malloc_result == malloc ); } + TEST_F( EntryTest, CopyMallocFailureOnSizeOverflow ) { + const struct stumpless_entry *result; + + result = stumpless_copy_entry( large_entry ); + EXPECT_NULL( result ); + EXPECT_ERROR_ID_EQ( STUMPLESS_MEMORY_ALLOCATION_FAILURE ); + } + TEST_F( EntryTest, CopyReallocFailure ) { const struct stumpless_entry *result; void * (*set_realloc_result)(void *, size_t); From c4631087baae607992da3dc9ebe64a48f2ff9930 Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Sun, 6 Oct 2024 03:48:41 +0530 Subject: [PATCH 4/5] Free large_entry in tests --- test/function/entry.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/function/entry.cpp b/test/function/entry.cpp index 1450824e0..790a9efd7 100644 --- a/test/function/entry.cpp +++ b/test/function/entry.cpp @@ -66,6 +66,7 @@ namespace { struct stumpless_param *param_1_1 = NULL; struct stumpless_entry *nil_entry = NULL; struct stumpless_entry *large_entry = NULL; + size_t large_entry_ec; virtual void SetUp( void ) { @@ -91,12 +92,15 @@ namespace { basic_app_name, basic_msgid, basic_message ); + large_entry_ec = large_entry->element_count; large_entry->element_count = SIZE_MAX; } virtual void TearDown( void ){ stumpless_destroy_entry_and_contents( basic_entry ); + large_entry->element_count = large_entry_ec; + stumpless_destroy_entry_and_contents(large_entry); stumpless_destroy_entry_only( nil_entry ); stumpless_free_all( ); } From d8c8e057ec04b9add0e98e55c28876c2583bdabc Mon Sep 17 00:00:00 2001 From: Aadithyaa Eeswaran Date: Sun, 6 Oct 2024 21:05:49 +0530 Subject: [PATCH 5/5] Cleanup --- src/memory.c | 4 ++-- test/function/entry.cpp | 41 +++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/memory.c b/src/memory.c index 3dfcd6613..ec334e845 100644 --- a/src/memory.c +++ b/src/memory.c @@ -130,8 +130,8 @@ realloc_mem( const void *mem, size_t size ) { 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; + 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 790a9efd7..6c1370929 100644 --- a/test/function/entry.cpp +++ b/test/function/entry.cpp @@ -65,8 +65,6 @@ namespace { const char *param_1_1_value = "basic-value"; struct stumpless_param *param_1_1 = NULL; struct stumpless_entry *nil_entry = NULL; - struct stumpless_entry *large_entry = NULL; - size_t large_entry_ec; virtual void SetUp( void ) { @@ -87,20 +85,12 @@ namespace { nil_entry = create_nil_entry(); - large_entry = stumpless_new_entry_str( STUMPLESS_FACILITY_USER, - STUMPLESS_SEVERITY_INFO, - basic_app_name, - basic_msgid, - basic_message ); - large_entry_ec = large_entry->element_count; - large_entry->element_count = SIZE_MAX; + } virtual void TearDown( void ){ stumpless_destroy_entry_and_contents( basic_entry ); - large_entry->element_count = large_entry_ec; - stumpless_destroy_entry_and_contents(large_entry); stumpless_destroy_entry_only( nil_entry ); stumpless_free_all( ); } @@ -371,13 +361,6 @@ namespace { EXPECT_TRUE( set_malloc_result == malloc ); } - TEST_F( EntryTest, CopyMallocFailureOnSizeOverflow ) { - const struct stumpless_entry *result; - - result = stumpless_copy_entry( large_entry ); - EXPECT_NULL( result ); - EXPECT_ERROR_ID_EQ( STUMPLESS_MEMORY_ALLOCATION_FAILURE ); - } TEST_F( EntryTest, CopyReallocFailure ) { const struct stumpless_entry *result; @@ -2911,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); + } + }