From 30865c9ccedaa32ddb036e87a8ebb52c3f18f695 Mon Sep 17 00:00:00 2001 From: Ewerton Scaboro da Silva Date: Thu, 1 Feb 2024 16:37:53 -0800 Subject: [PATCH] Use safe math in message.c (#452) --- src/message.c | 208 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 127 insertions(+), 81 deletions(-) diff --git a/src/message.c b/src/message.c index 55e072d6..6073d1fe 100644 --- a/src/message.c +++ b/src/message.c @@ -7,6 +7,7 @@ #include "azure_macro_utils/macro_utils.h" #include "azure_c_shared_utility/gballoc.h" #include "azure_c_shared_utility/xlogging.h" +#include "azure_c_shared_utility/safe_math.h" #include "azure_uamqp_c/amqp_definitions.h" #include "azure_uamqp_c/message.h" #include "azure_uamqp_c/amqpvalue.h" @@ -232,78 +233,100 @@ MESSAGE_HANDLE message_clone(MESSAGE_HANDLE source_message) if ((result != NULL) && (source_message->body_amqp_data_count > 0)) { - size_t i; + size_t calloc_size = safe_multiply_size_t(source_message->body_amqp_data_count, sizeof(BODY_AMQP_DATA)); - result->body_amqp_data_items = (BODY_AMQP_DATA*)calloc(1, (source_message->body_amqp_data_count * sizeof(BODY_AMQP_DATA))); - if (result->body_amqp_data_items == NULL) + if (calloc_size == SIZE_MAX) { - /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ - LogError("Cannot allocate memory for body data sections"); - message_destroy(result); + LogError("Invalid size for body_amqp_data_items"); result = NULL; } else { - for (i = 0; i < source_message->body_amqp_data_count; i++) + result->body_amqp_data_items = (BODY_AMQP_DATA*)calloc(1, calloc_size); + + if (result->body_amqp_data_items == NULL) { - result->body_amqp_data_items[i].body_data_section_length = source_message->body_amqp_data_items[i].body_data_section_length; + /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ + LogError("Cannot allocate memory for body data sections"); + message_destroy(result); + result = NULL; + } + else + { + size_t i; - /* Codes_SRS_MESSAGE_01_011: [If an AMQP data has been set as message body on the source message it shall be cloned by allocating memory for the binary payload.] */ - result->body_amqp_data_items[i].body_data_section_bytes = (unsigned char*)malloc(source_message->body_amqp_data_items[i].body_data_section_length); - if (result->body_amqp_data_items[i].body_data_section_bytes == NULL) + for (i = 0; i < source_message->body_amqp_data_count; i++) { - LogError("Cannot allocate memory for body data section %u", (unsigned int)i); - break; + result->body_amqp_data_items[i].body_data_section_length = source_message->body_amqp_data_items[i].body_data_section_length; + + /* Codes_SRS_MESSAGE_01_011: [If an AMQP data has been set as message body on the source message it shall be cloned by allocating memory for the binary payload.] */ + result->body_amqp_data_items[i].body_data_section_bytes = (unsigned char*)malloc(source_message->body_amqp_data_items[i].body_data_section_length); + if (result->body_amqp_data_items[i].body_data_section_bytes == NULL) + { + LogError("Cannot allocate memory for body data section %u", (unsigned int)i); + break; + } + else + { + (void)memcpy(result->body_amqp_data_items[i].body_data_section_bytes, source_message->body_amqp_data_items[i].body_data_section_bytes, result->body_amqp_data_items[i].body_data_section_length); + } } - else + + result->body_amqp_data_count = i; + if (i < source_message->body_amqp_data_count) { - (void)memcpy(result->body_amqp_data_items[i].body_data_section_bytes, source_message->body_amqp_data_items[i].body_data_section_bytes, result->body_amqp_data_items[i].body_data_section_length); + /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ + message_destroy(result); + result = NULL; } } - - result->body_amqp_data_count = i; - if (i < source_message->body_amqp_data_count) - { - /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ - message_destroy(result); - result = NULL; - } } } if ((result != NULL) && (source_message->body_amqp_sequence_count > 0)) { - size_t i; + size_t calloc_size = safe_multiply_size_t(source_message->body_amqp_sequence_count, sizeof(AMQP_VALUE)); - result->body_amqp_sequence_items = (AMQP_VALUE*)calloc(1, (source_message->body_amqp_sequence_count * sizeof(AMQP_VALUE))); - if (result->body_amqp_sequence_items == NULL) + if (calloc_size == SIZE_MAX) { - /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ - LogError("Cannot allocate memory for body AMQP sequences"); + LogError("Invalid size for body_amqp_sequence_items"); message_destroy(result); result = NULL; } else { - for (i = 0; i < source_message->body_amqp_sequence_count; i++) - { - /* Codes_SRS_MESSAGE_01_160: [ If AMQP sequences are set as AMQP body they shall be cloned by calling `amqpvalue_clone`. ] */ - result->body_amqp_sequence_items[i] = amqpvalue_clone(source_message->body_amqp_sequence_items[i]); - if (result->body_amqp_sequence_items[i] == NULL) - { - LogError("Cannot clone AMQP sequence %u", (unsigned int)i); - break; - } - } - - result->body_amqp_sequence_count = i; - if (i < source_message->body_amqp_sequence_count) + result->body_amqp_sequence_items = (AMQP_VALUE*)calloc(1, calloc_size); + if (result->body_amqp_sequence_items == NULL) { /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ + LogError("Cannot allocate memory for body AMQP sequences"); message_destroy(result); result = NULL; } - } + else + { + size_t i; + + for (i = 0; i < source_message->body_amqp_sequence_count; i++) + { + /* Codes_SRS_MESSAGE_01_160: [ If AMQP sequences are set as AMQP body they shall be cloned by calling `amqpvalue_clone`. ] */ + result->body_amqp_sequence_items[i] = amqpvalue_clone(source_message->body_amqp_sequence_items[i]); + if (result->body_amqp_sequence_items[i] == NULL) + { + LogError("Cannot clone AMQP sequence %u", (unsigned int)i); + break; + } + } + + result->body_amqp_sequence_count = i; + if (i < source_message->body_amqp_sequence_count) + { + /* Codes_SRS_MESSAGE_01_012: [ If any cloning operation for the members of the source message fails, then `message_clone` shall fail and return NULL. ]*/ + message_destroy(result); + result = NULL; + } + } + } } if ((result != NULL) && (source_message->body_amqp_value != NULL)) @@ -1017,45 +1040,56 @@ int message_add_body_amqp_data(MESSAGE_HANDLE message, BINARY_DATA amqp_data) } else { - /* Codes_SRS_MESSAGE_01_086: [ `message_add_body_amqp_data` shall add the contents of `amqp_data` to the list of AMQP data values for the body of the message identified by `message`. ]*/ - BODY_AMQP_DATA* new_body_amqp_data_items = (BODY_AMQP_DATA*)realloc(message->body_amqp_data_items, sizeof(BODY_AMQP_DATA) * (message->body_amqp_data_count + 1)); - if (new_body_amqp_data_items == NULL) + size_t realloc_size = safe_add_size_t(message->body_amqp_data_count, 1); + realloc_size = safe_multiply_size_t(sizeof(BODY_AMQP_DATA), realloc_size); + + if (realloc_size == SIZE_MAX) { - /* Codes_SRS_MESSAGE_01_153: [ If allocating memory to store the added AMQP data fails, `message_add_body_amqp_data` shall fail and return a non-zero value. ]*/ - LogError("Cannot allocate memory for body AMQP data items"); + LogError("Invalid size for new_body_amqp_data_items"); result = MU_FAILURE; } else { - message->body_amqp_data_items = new_body_amqp_data_items; - - if (amqp_data.length == 0) + /* Codes_SRS_MESSAGE_01_086: [ `message_add_body_amqp_data` shall add the contents of `amqp_data` to the list of AMQP data values for the body of the message identified by `message`. ]*/ + BODY_AMQP_DATA* new_body_amqp_data_items = (BODY_AMQP_DATA*)realloc(message->body_amqp_data_items, realloc_size); + if (new_body_amqp_data_items == NULL) { - message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = NULL; - message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_length = 0; - message->body_amqp_data_count++; - - /* Codes_SRS_MESSAGE_01_087: [ On success it shall return 0. ]*/ - result = 0; + /* Codes_SRS_MESSAGE_01_153: [ If allocating memory to store the added AMQP data fails, `message_add_body_amqp_data` shall fail and return a non-zero value. ]*/ + LogError("Cannot allocate memory for body AMQP data items"); + result = MU_FAILURE; } else { - message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = (unsigned char*)malloc(amqp_data.length); - if (message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes == NULL) - { - /* Codes_SRS_MESSAGE_01_153: [ If allocating memory to store the added AMQP data fails, `message_add_body_amqp_data` shall fail and return a non-zero value. ]*/ - LogError("Cannot allocate memory for body AMQP data to be added"); - result = MU_FAILURE; - } - else + message->body_amqp_data_items = new_body_amqp_data_items; + + if (amqp_data.length == 0) { - message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_length = amqp_data.length; - (void)memcpy(message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes, amqp_data.bytes, amqp_data.length); + message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = NULL; + message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_length = 0; message->body_amqp_data_count++; /* Codes_SRS_MESSAGE_01_087: [ On success it shall return 0. ]*/ result = 0; } + else + { + message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = (unsigned char*)malloc(amqp_data.length); + if (message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes == NULL) + { + /* Codes_SRS_MESSAGE_01_153: [ If allocating memory to store the added AMQP data fails, `message_add_body_amqp_data` shall fail and return a non-zero value. ]*/ + LogError("Cannot allocate memory for body AMQP data to be added"); + result = MU_FAILURE; + } + else + { + message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_length = amqp_data.length; + (void)memcpy(message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes, amqp_data.bytes, amqp_data.length); + message->body_amqp_data_count++; + + /* Codes_SRS_MESSAGE_01_087: [ On success it shall return 0. ]*/ + result = 0; + } + } } } } @@ -1250,33 +1284,45 @@ int message_add_body_amqp_sequence(MESSAGE_HANDLE message, AMQP_VALUE sequence_l } else { - AMQP_VALUE* new_body_amqp_sequence_items = (AMQP_VALUE*)realloc(message->body_amqp_sequence_items, sizeof(AMQP_VALUE) * (message->body_amqp_sequence_count + 1)); - if (new_body_amqp_sequence_items == NULL) + size_t realloc_size = safe_add_size_t(message->body_amqp_sequence_count, 1); + realloc_size = safe_multiply_size_t(sizeof(AMQP_VALUE), realloc_size); + + if (realloc_size == SIZE_MAX) { - /* Codes_SRS_MESSAGE_01_158: [ If allocating memory in order to store the sequence fails, `message_add_body_amqp_sequence` shall fail and return a non-zero value. ]*/ - LogError("Cannot allocate enough memory for sequence items"); + LogError("Invalid size for new_body_amqp_sequence_items"); result = MU_FAILURE; } else { - message->body_amqp_sequence_items = new_body_amqp_sequence_items; + AMQP_VALUE* new_body_amqp_sequence_items = (AMQP_VALUE*)realloc(message->body_amqp_sequence_items, realloc_size); - /* Codes_SRS_MESSAGE_01_110: [ `message_add_body_amqp_sequence` shall add the contents of `sequence` to the list of AMQP sequences for the body of the message identified by `message`. ]*/ - /* Codes_SRS_MESSAGE_01_156: [ The AMQP sequence shall be cloned by calling `amqpvalue_clone`. ]*/ - message->body_amqp_sequence_items[message->body_amqp_sequence_count] = amqpvalue_clone(sequence_list); - if (message->body_amqp_sequence_items[message->body_amqp_sequence_count] == NULL) + if (new_body_amqp_sequence_items == NULL) { - /* Codes_SRS_MESSAGE_01_157: [ If `amqpvalue_clone` fails, `message_add_body_amqp_sequence` shall fail and return a non-zero value. ]*/ - LogError("Cloning sequence failed"); + /* Codes_SRS_MESSAGE_01_158: [ If allocating memory in order to store the sequence fails, `message_add_body_amqp_sequence` shall fail and return a non-zero value. ]*/ + LogError("Cannot allocate enough memory for sequence items"); result = MU_FAILURE; } else { - /* Codes_SRS_MESSAGE_01_114: [ If adding the AMQP sequence fails, the previous value shall be preserved. ]*/ - message->body_amqp_sequence_count++; + message->body_amqp_sequence_items = new_body_amqp_sequence_items; - /* Codes_SRS_MESSAGE_01_111: [ On success it shall return 0. ]*/ - result = 0; + /* Codes_SRS_MESSAGE_01_110: [ `message_add_body_amqp_sequence` shall add the contents of `sequence` to the list of AMQP sequences for the body of the message identified by `message`. ]*/ + /* Codes_SRS_MESSAGE_01_156: [ The AMQP sequence shall be cloned by calling `amqpvalue_clone`. ]*/ + message->body_amqp_sequence_items[message->body_amqp_sequence_count] = amqpvalue_clone(sequence_list); + if (message->body_amqp_sequence_items[message->body_amqp_sequence_count] == NULL) + { + /* Codes_SRS_MESSAGE_01_157: [ If `amqpvalue_clone` fails, `message_add_body_amqp_sequence` shall fail and return a non-zero value. ]*/ + LogError("Cloning sequence failed"); + result = MU_FAILURE; + } + else + { + /* Codes_SRS_MESSAGE_01_114: [ If adding the AMQP sequence fails, the previous value shall be preserved. ]*/ + message->body_amqp_sequence_count++; + + /* Codes_SRS_MESSAGE_01_111: [ On success it shall return 0. ]*/ + result = 0; + } } } }