From d74b08f8b37a44e31d8fa0e62ff0d62ce6e62cd0 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 16 Aug 2024 20:49:06 -0700 Subject: [PATCH] AMQP static analysis fixes (#5918) * AMQP static analysis fixes? --- .../vendor/azure-uamqp-c/src/amqpvalue.c | 12 ++++++++ .../vendor/azure-uamqp-c/src/connection.c | 9 ++++++ .../vendor/azure-uamqp-c/src/message.c | 30 +++++++++++++++++-- .../vendor/azure-uamqp-c/src/message_sender.c | 9 ++++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue.c index 3d9188d86..586e0f7bc 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue.c @@ -1419,6 +1419,10 @@ int amqpvalue_set_list_item(AMQP_VALUE value, uint32_t index, AMQP_VALUE list_it AMQP_VALUE* new_list; size_t realloc_size = safe_add_size_t((size_t)index, 1); realloc_size = safe_multiply_size_t(realloc_size, sizeof(AMQP_VALUE)); +#if defined(_MSC_VER) + __analysis_assume(realloc_size == ((size_t) index + 1) * sizeof(AMQP_VALUE)); +#endif + if (realloc_size == SIZE_MAX || (new_list = (AMQP_VALUE*)realloc(value_data->value.list_value.items, realloc_size)) == NULL) { @@ -1432,9 +1436,17 @@ int amqpvalue_set_list_item(AMQP_VALUE value, uint32_t index, AMQP_VALUE list_it uint32_t i; value_data->value.list_value.items = new_list; +#if defined(_MSC_VER) + __analysis_assume(value_data->value.list_value.count < index); + __analysis_assume(value_data->value.list_value.count < index); + __analysis_assume((realloc_size / sizeof(AMQP_VALUE)) > index); +#endif for (i = value_data->value.list_value.count; i < index; i++) { +#if defined(_MSC_VER) + __analysis_assume(i < index); +#endif new_list[i] = amqpvalue_create_null(); if (new_list[i] == NULL) { diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/connection.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/connection.c index c1d7c29a9..8d4f470a7 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/connection.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/connection.c @@ -2218,6 +2218,12 @@ ENDPOINT_HANDLE connection_create_endpoint(CONNECTION_HANDLE connection) * the endpoints list, so that it can be tracked.] */ size_t realloc_size = safe_add_size_t((size_t)connection->endpoint_count, 1); realloc_size = safe_multiply_size_t(realloc_size, sizeof(ENDPOINT_HANDLE)); +#if defined(_MSC_VER) + __analysis_assume(realloc_size == (connection->endpoint_count+1) * sizeof(ENDPOINT_HANDLE)); + __analysis_assume((realloc_size / sizeof(ENDPOINT_HANDLE)) > connection->endpoint_count); + __analysis_assume(connection->endpoint_count < (realloc_size / sizeof(ENDPOINT_HANDLE))); + __analysis_assume((connection->endpoint_count*sizeof(ENDPOINT_HANDLE)) < realloc_size); +#endif if (realloc_size == SIZE_MAX || (new_endpoints = (ENDPOINT_HANDLE*)realloc(connection->endpoints, realloc_size)) == NULL) { @@ -2233,6 +2239,9 @@ ENDPOINT_HANDLE connection_create_endpoint(CONNECTION_HANDLE connection) { // Insert the new endpoint at the end of the set of endpoints. connection->endpoints = new_endpoints; +#if defined(_MSC_VER) + __analysis_assume(connection->endpoint_count < (realloc_size / sizeof(ENDPOINT_HANDLE))); +#endif connection->endpoints[connection->endpoint_count] = result; connection->endpoint_count++; diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message.c index 8e9bce06c..8c4261c1a 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message.c @@ -247,6 +247,9 @@ MESSAGE_HANDLE message_clone(MESSAGE_HANDLE source_message) if ((result != NULL) && (source_message->body_amqp_data_count > 0)) { size_t calloc_size = safe_multiply_size_t(source_message->body_amqp_data_count, sizeof(BODY_AMQP_DATA)); +#if defined(_MSC_VER) + __analysis_assume(calloc_size == (source_message->body_amqp_data_count+1)*sizeof(BODY_AMQP_DATA)); +#endif if (calloc_size == SIZE_MAX) { @@ -285,7 +288,7 @@ MESSAGE_HANDLE message_clone(MESSAGE_HANDLE source_message) } 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); + (void)memcpy(result->body_amqp_data_items[i].body_data_section_bytes, source_message->body_amqp_data_items[i].body_data_section_bytes, source_message->body_amqp_data_items[i].body_data_section_length); } } @@ -304,6 +307,9 @@ MESSAGE_HANDLE message_clone(MESSAGE_HANDLE source_message) if ((result != NULL) && (source_message->body_amqp_sequence_count > 0)) { size_t calloc_size = safe_multiply_size_t(source_message->body_amqp_sequence_count, sizeof(AMQP_VALUE)); +#if defined(_MSC_VER) + __analysis_assume(calloc_size == (source_message->body_amqp_sequence_count+1) * sizeof(AMQP_VALUE)); +#endif if (calloc_size == SIZE_MAX) { @@ -1068,6 +1074,12 @@ int message_add_body_amqp_data(MESSAGE_HANDLE message, BINARY_DATA amqp_data) { 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 defined(_MSC_VER) + __analysis_assume(realloc_size == (message->body_amqp_data_count+1) * sizeof(BODY_AMQP_DATA)); + __analysis_assume(realloc_size / sizeof(BODY_AMQP_DATA) > message->body_amqp_data_count); + __analysis_assume(message->body_amqp_data_count < realloc_size / sizeof(BODY_AMQP_DATA)); +#endif + if (realloc_size == SIZE_MAX) { @@ -1091,6 +1103,9 @@ int message_add_body_amqp_data(MESSAGE_HANDLE message, BINARY_DATA amqp_data) { message->body_amqp_data_items = new_body_amqp_data_items; +#if defined(_MSC_VER) + __analysis_assume(realloc_size > message->body_amqp_data_count * sizeof(BODY_AMQP_DATA)); +#endif if (amqp_data.length == 0) { message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = NULL; @@ -1102,7 +1117,8 @@ int message_add_body_amqp_data(MESSAGE_HANDLE message, BINARY_DATA amqp_data) } else { - message->body_amqp_data_items[message->body_amqp_data_count].body_data_section_bytes = (unsigned char*)malloc(amqp_data.length); + 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 @@ -1316,6 +1332,10 @@ int message_add_body_amqp_sequence(MESSAGE_HANDLE message, AMQP_VALUE sequence_l { 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 defined(_MSC_VER) + __analysis_assume(realloc_size == (message->body_amqp_sequence_count+1) * sizeof(AMQP_VALUE)); + __analysis_assume(message->body_amqp_sequence_count < realloc_size / sizeof(AMQP_VALUE)); +#endif if (realloc_size == SIZE_MAX) { @@ -1340,7 +1360,11 @@ int message_add_body_amqp_sequence(MESSAGE_HANDLE message, AMQP_VALUE sequence_l * 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 defined(_MSC_VER) + __analysis_assume(realloc_size > message->body_amqp_sequence_count * sizeof(AMQP_VALUE)); +#endif + 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, diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message_sender.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message_sender.c index 674116a7c..d219090a7 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message_sender.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/message_sender.c @@ -1078,6 +1078,11 @@ ASYNC_OPERATION_HANDLE messagesender_send_async(MESSAGE_SENDER_HANDLE message_se MESSAGE_WITH_CALLBACK* message_with_callback = GET_ASYNC_OPERATION_CONTEXT(MESSAGE_WITH_CALLBACK, result); size_t realloc_size = safe_add_size_t(message_sender->message_count, 1); realloc_size = safe_multiply_size_t(realloc_size, sizeof(ASYNC_OPERATION_HANDLE)); +#if defined(_MSC_VER) + __analysis_assume(realloc_size == (message_sender->message_count+1)* sizeof(ASYNC_OPERATION_HANDLE)); + __analysis_assume(message_sender->message_count < realloc_size / sizeof(ASYNC_OPERATION_HANDLE)); +#endif + if (realloc_size == SIZE_MAX || (new_messages = (ASYNC_OPERATION_HANDLE*)realloc(message_sender->messages, realloc_size)) == NULL) { @@ -1113,6 +1118,10 @@ ASYNC_OPERATION_HANDLE messagesender_send_async(MESSAGE_SENDER_HANDLE message_se message_with_callback->context = callback_context; message_with_callback->message_sender = message_sender; +#if defined(_MSC_VER) + __analysis_assume(message_sender->message_count < realloc_size / sizeof(ASYNC_OPERATION_HANDLE)); +#endif + message_sender->messages[message_sender->message_count] = result; message_sender->message_count++;