From 863e2b2b9186e5119e629706bbea263e3477319b Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 15 Aug 2024 22:28:08 -0700 Subject: [PATCH] Ported uAMQP changes to vendored copy of uAMQP (#5917) --- .../azure-uamqp-c/src/amqp_frame_codec.c | 10 +- .../vendor/azure-uamqp-c/src/amqpvalue.c | 55 ++-- .../azure-uamqp-c/src/amqpvalue_to_string.c | 8 +- .../vendor/azure-uamqp-c/src/connection.c | 64 +++-- .../azure-uamqp-c/src/header_detect_io.c | 7 +- .../vendor/azure-uamqp-c/src/link.c | 34 ++- .../vendor/azure-uamqp-c/src/message.c | 250 +++++++++++------- .../vendor/azure-uamqp-c/src/message_sender.c | 17 +- .../azure-uamqp-c/src/sasl_frame_codec.c | 15 +- .../vendor/azure-uamqp-c/src/session.c | 12 +- 10 files changed, 292 insertions(+), 180 deletions(-) diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqp_frame_codec.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqp_frame_codec.c index 5cbb42023..483b8b767 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqp_frame_codec.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqp_frame_codec.c @@ -11,6 +11,7 @@ #include "azure_uamqp_c/amqp_frame_codec.h" #include "azure_uamqp_c/frame_codec.h" #include "azure_uamqp_c/amqpvalue.h" +#include "azure_c_shared_utility/safe_math.h" typedef enum AMQP_FRAME_DECODE_STATE_TAG { @@ -274,10 +275,13 @@ int amqp_frame_codec_encode_frame(AMQP_FRAME_CODEC_HANDLE amqp_frame_codec, uint } else { - PAYLOAD* new_payloads = (PAYLOAD*)calloc(1, (sizeof(PAYLOAD) * (payload_count + 1))); - if (new_payloads == NULL) + PAYLOAD* new_payloads = NULL; + size_t calloc_size = safe_add_size_t(payload_count, 1); + calloc_size = safe_multiply_size_t(calloc_size, sizeof(PAYLOAD)); + if (calloc_size == SIZE_MAX || + (new_payloads = (PAYLOAD*)calloc(1, calloc_size)) == NULL) { - LogError("Could not allocate frame payloads"); + LogError("Could not allocate frame payloads, size:%zu", calloc_size); result = MU_FAILURE; } else 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 d3d040d00..3d9188d86 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 @@ -1278,11 +1278,12 @@ int amqpvalue_set_list_item_count(AMQP_VALUE value, uint32_t list_size) AMQP_VALUE* new_list; /* Codes_SRS_AMQPVALUE_01_152: [amqpvalue_set_list_item_count shall resize an AMQP list.] */ - new_list = (AMQP_VALUE*)realloc(value_data->value.list_value.items, list_size * sizeof(AMQP_VALUE)); - if (new_list == NULL) + size_t realloc_size = safe_multiply_size_t(list_size, sizeof(AMQP_VALUE)); + if (realloc_size == SIZE_MAX || + (new_list = (AMQP_VALUE*)realloc(value_data->value.list_value.items, realloc_size)) == NULL) { /* Codes_SRS_AMQPVALUE_01_154: [If allocating memory for the list according to the new size fails, then amqpvalue_set_list_item_count shall return a non-zero value, while preserving the existing list contents.] */ - LogError("Could not reallocate list memory"); + LogError("Could not reallocate list memory, size:%zu", realloc_size); result = MU_FAILURE; } else @@ -1415,11 +1416,14 @@ int amqpvalue_set_list_item(AMQP_VALUE value, uint32_t index, AMQP_VALUE list_it { if (index >= value_data->value.list_value.count) { - AMQP_VALUE* new_list = (AMQP_VALUE*)realloc(value_data->value.list_value.items, ((size_t)index + 1) * sizeof(AMQP_VALUE)); - if (new_list == NULL) + 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 (realloc_size == SIZE_MAX || + (new_list = (AMQP_VALUE*)realloc(value_data->value.list_value.items, realloc_size)) == NULL) { /* Codes_SRS_AMQPVALUE_01_170: [When amqpvalue_set_list_item fails due to not being able to clone the item or grow the list, the list shall not be altered.] */ - LogError("Could not reallocate list storage"); + LogError("Could not reallocate list storage, size:%zu", realloc_size); amqpvalue_destroy(cloned_item); result = MU_FAILURE; } @@ -1642,13 +1646,16 @@ int amqpvalue_set_map_value(AMQP_VALUE map, AMQP_VALUE key, AMQP_VALUE value) } else { - AMQP_MAP_KEY_VALUE_PAIR* new_pairs = (AMQP_MAP_KEY_VALUE_PAIR*)realloc(value_data->value.map_value.pairs, ((size_t)value_data->value.map_value.pair_count + 1) * sizeof(AMQP_MAP_KEY_VALUE_PAIR)); - if (new_pairs == NULL) + AMQP_MAP_KEY_VALUE_PAIR* new_pairs; + size_t realloc_size = safe_add_size_t((size_t)value_data->value.map_value.pair_count, 1); + realloc_size = safe_multiply_size_t(realloc_size, sizeof(AMQP_MAP_KEY_VALUE_PAIR)); + if (realloc_size == SIZE_MAX || + (new_pairs = (AMQP_MAP_KEY_VALUE_PAIR*)realloc(value_data->value.map_value.pairs, realloc_size)) == NULL) { /* Codes_SRS_AMQPVALUE_01_186: [If allocating memory to hold a new key/value pair fails, amqpvalue_set_map_value shall fail and return a non-zero value.] */ amqpvalue_destroy(cloned_key); amqpvalue_destroy(cloned_value); - LogError("Could not reallocate memory for map"); + LogError("Could not reallocate memory for new_pairs map, size:%zu", realloc_size); result = MU_FAILURE; } else @@ -1947,13 +1954,16 @@ int amqpvalue_add_array_item(AMQP_VALUE value, AMQP_VALUE array_item_value) } else { - AMQP_VALUE* new_array = (AMQP_VALUE*)realloc(value_data->value.array_value.items, ((size_t)value_data->value.array_value.count + 1) * sizeof(AMQP_VALUE)); - if (new_array == NULL) + AMQP_VALUE* new_array; + size_t realloc_size = safe_add_size_t((size_t)value_data->value.array_value.count, 1); + realloc_size = safe_multiply_size_t(realloc_size, sizeof(AMQP_VALUE)); + if (realloc_size == SIZE_MAX || + (new_array = (AMQP_VALUE*)realloc(value_data->value.array_value.items, realloc_size)) == NULL) { /* Codes_SRS_AMQPVALUE_01_423: [ When `amqpvalue_add_array_item` fails due to not being able to clone the item or grow the array, the array shall not be altered. ] */ /* Codes_SRS_AMQPVALUE_01_424: [ If growing the array fails, then `amqpvalue_add_array_item` shall fail and return a non-zero value. ] */ amqpvalue_destroy(cloned_item); - LogError("Cannot resize array"); + LogError("Cannot resize array, size:%zu", realloc_size); result = MU_FAILURE; } else @@ -6389,10 +6399,11 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder else { uint32_t i; - internal_decoder_data->decode_to_value->value.list_value.items = (AMQP_VALUE*)calloc(1, (sizeof(AMQP_VALUE) * internal_decoder_data->decode_to_value->value.list_value.count)); - if (internal_decoder_data->decode_to_value->value.list_value.items == NULL) + size_t calloc_size = safe_multiply_size_t(sizeof(AMQP_VALUE), internal_decoder_data->decode_to_value->value.list_value.count); + if (calloc_size == SIZE_MAX || + (internal_decoder_data->decode_to_value->value.list_value.items = (AMQP_VALUE*)calloc(1, calloc_size)) == NULL) { - LogError("Could not allocate memory for decoded list value"); + LogError("Could not allocate memory for decoded list value, size:%zu", calloc_size); result = MU_FAILURE; } else @@ -6855,10 +6866,11 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder else { uint32_t i; - internal_decoder_data->decode_to_value->value.array_value.items = (AMQP_VALUE*)calloc(1, (sizeof(AMQP_VALUE) * internal_decoder_data->decode_to_value->value.array_value.count)); - if (internal_decoder_data->decode_to_value->value.array_value.items == NULL) + size_t calloc_size = safe_multiply_size_t(sizeof(AMQP_VALUE), internal_decoder_data->decode_to_value->value.array_value.count); + if (calloc_size == SIZE_MAX || + (internal_decoder_data->decode_to_value->value.array_value.items = (AMQP_VALUE*)calloc(1, calloc_size)) == NULL) { - LogError("Could not allocate memory for array items"); + LogError("Could not allocate memory for array items, size:%zu", calloc_size); result = MU_FAILURE; } else @@ -6889,10 +6901,11 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder else { uint32_t i; - internal_decoder_data->decode_to_value->value.array_value.items = (AMQP_VALUE*)calloc(1, (sizeof(AMQP_VALUE) * internal_decoder_data->decode_to_value->value.array_value.count)); - if (internal_decoder_data->decode_to_value->value.array_value.items == NULL) + size_t calloc_size = safe_multiply_size_t(sizeof(AMQP_VALUE), internal_decoder_data->decode_to_value->value.array_value.count); + if (calloc_size == SIZE_MAX || + (internal_decoder_data->decode_to_value->value.array_value.items = (AMQP_VALUE*)calloc(1, calloc_size)) == NULL) { - LogError("Could not allocate memory for array items"); + LogError("Could not allocate memory for array items, size:%zu", calloc_size); result = MU_FAILURE; } else diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue_to_string.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue_to_string.c index 964ce46e9..b5148a414 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue_to_string.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/amqpvalue_to_string.c @@ -13,6 +13,7 @@ #include "azure_c_shared_utility/uuid.h" #include "azure_uamqp_c/amqpvalue_to_string.h" #include "azure_uamqp_c/amqpvalue.h" +#include "azure_c_shared_utility/safe_math.h" #if _WIN32 /* The MS runtime does not have snprintf */ @@ -35,10 +36,11 @@ static int string_concat(char** string, const char* to_concat) src_length = 0; } - new_string = (char*)realloc(*string, src_length + length); - if (new_string == NULL) + size_t realloc_size = safe_add_size_t(src_length, length); + if (realloc_size == SIZE_MAX || + (new_string = (char*)realloc(*string, realloc_size)) == NULL) { - LogError("Cannot allocate memory for the new string"); + LogError("Cannot allocate memory for the new string, size:%zu", realloc_size); result = MU_FAILURE; } else 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 a25a3d99d..c1d7c29a9 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 @@ -9,7 +9,6 @@ #include "azure_c_shared_utility/xio.h" #include "azure_c_shared_utility/xlogging.h" #include "azure_c_shared_utility/tickcounter.h" -#include "azure_c_shared_utility/safe_math.h" #include "azure_uamqp_c/connection.h" #include "azure_uamqp_c/frame_codec.h" @@ -17,6 +16,7 @@ #include "azure_uamqp_c/amqp_frame_codec.h" #include "azure_uamqp_c/amqp_definitions.h" #include "azure_uamqp_c/amqpvalue_to_string.h" +#include "azure_c_shared_utility/safe_math.h" /* Requirements satisfied by the virtue of implementing the ISO:*/ /* Codes_S_R_S_CONNECTION_01_088: [Any data appearing beyond the protocol header MUST match the version indicated by the protocol header.] */ @@ -1572,26 +1572,17 @@ CONNECTION_HANDLE connection_create2(XIO_HANDLE xio, const char* hostname, const connection->on_new_endpoint = on_new_endpoint; connection->on_new_endpoint_callback_context = callback_context; - connection->on_connection_close_received_event_subscription - .on_connection_close_received - = NULL; - connection->on_connection_close_received_event_subscription.context - = NULL; + connection->on_connection_close_received_event_subscription.on_connection_close_received = NULL; + connection->on_connection_close_received_event_subscription.context = NULL; connection->on_io_error = on_io_error; connection->on_io_error_callback_context = on_io_error_context; - connection->on_connection_state_changed - = on_connection_state_changed; - connection->on_connection_state_changed_callback_context - = on_connection_state_changed_context; + connection->on_connection_state_changed = on_connection_state_changed; + connection->on_connection_state_changed_callback_context = on_connection_state_changed_context; - if (tickcounter_get_current_ms( - connection->tick_counter, - &connection->last_frame_received_time) - != 0) + if (tickcounter_get_current_ms(connection->tick_counter, &connection->last_frame_received_time) != 0) { - LogError( - "Could not retrieve time for last frame received time"); + LogError("Could not retrieve time for last frame received time"); tickcounter_destroy(connection->tick_counter); free(connection->container_id); free(connection->host_name); @@ -1602,8 +1593,7 @@ CONNECTION_HANDLE connection_create2(XIO_HANDLE xio, const char* hostname, const } else { - connection->last_frame_sent_time - = connection->last_frame_received_time; + connection->last_frame_sent_time = connection->last_frame_received_time; /* Codes_S_R_S_CONNECTION_01_072: [When connection_create * succeeds, the state of the connection shall be @@ -2226,15 +2216,15 @@ ENDPOINT_HANDLE connection_create_endpoint(CONNECTION_HANDLE connection) /* Codes_S_R_S_CONNECTION_01_197: [The newly created endpoint shall be added to * the endpoints list, so that it can be tracked.] */ - new_endpoints = (ENDPOINT_HANDLE*)realloc( - connection->endpoints, - sizeof(ENDPOINT_HANDLE) * ((size_t)connection->endpoint_count + 1)); - if (new_endpoints == NULL) + 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 (realloc_size == SIZE_MAX + || (new_endpoints = (ENDPOINT_HANDLE*)realloc(connection->endpoints, realloc_size)) == NULL) { /* Tests_S_R_S_CONNECTION_01_198: [If adding the endpoint to the endpoints - * list tracked by the connection fails, connection_create_endpoint shall - * fail and return NULL.] */ - LogError("Cannot reallocate memory for connection endpoints"); + * list tracked by the connection fails, connection_create_endpoint shall fail + * and return NULL.] */ + LogError("Cannot reallocate memory for connection endpoints, size:%zu", realloc_size); channel_table_free(&connection->channel_table, outgoing_channel); free(result); result = NULL; @@ -2366,12 +2356,28 @@ void connection_destroy_endpoint(ENDPOINT_HANDLE endpoint) { ENDPOINT_HANDLE* new_endpoints; - if ((connection->endpoint_count - i - 1) > 0) + size_t new_count = safe_subtract_size_t(safe_subtract_size_t(connection->endpoint_count, i), 1); + if (new_count > 0) { - (void)memmove(connection->endpoints + i, connection->endpoints + i + 1, sizeof(ENDPOINT_HANDLE) * (connection->endpoint_count - i - 1)); + size_t memmove_size = safe_multiply_size_t(safe_subtract_size_t(safe_subtract_size_t(connection->endpoint_count, i), 1), sizeof(ENDPOINT_HANDLE)); + if (memmove_size != SIZE_MAX) + { + (void)memmove(connection->endpoints + i, connection->endpoints + i + 1, memmove_size); + } + else + { + LogError("Cannot memmove endpoints, size:%zu", memmove_size); + } } - new_endpoints = (ENDPOINT_HANDLE*)realloc(connection->endpoints, (connection->endpoint_count - 1) * sizeof(ENDPOINT_HANDLE)); - if (new_endpoints != NULL) + + size_t realloc_size = safe_subtract_size_t(connection->endpoint_count, 1); + realloc_size = safe_multiply_size_t(realloc_size, sizeof(ENDPOINT_HANDLE)); + if (realloc_size == SIZE_MAX || + (new_endpoints = (ENDPOINT_HANDLE*)realloc(connection->endpoints, realloc_size)) == NULL) + { + LogError("Memory realloc failed, size:%zu", realloc_size); + } + else { connection->endpoints = new_endpoints; } diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/header_detect_io.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/header_detect_io.c index e73b90acd..f8972da7d 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/header_detect_io.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/header_detect_io.c @@ -10,6 +10,7 @@ #include "azure_c_shared_utility/singlylinkedlist.h" #include "azure_uamqp_c/header_detect_io.h" #include "azure_uamqp_c/server_protocol_io.h" +#include "azure_c_shared_utility/safe_math.h" static const unsigned char amqp_header_bytes[] = { 'A', 'M', 'Q', 'P', 0, 1, 0, 0 }; static const unsigned char sasl_amqp_header_bytes[] = { 'A', 'M', 'Q', 'P', 3, 1, 0, 0 }; @@ -529,9 +530,11 @@ static CONCRETE_IO_HANDLE header_detect_io_create(void* io_create_parameters) else { /* Codes_SRS_HEADER_DETECT_IO_01_009: [ The `header_detect_entries` array shall be copied so that it can be later used when detecting which header was received. ]*/ - result->header_detect_entries = (INTERNAL_HEADER_DETECT_ENTRY*)calloc(1, (header_detect_io_config->header_detect_entry_count * sizeof(INTERNAL_HEADER_DETECT_ENTRY))); - if (result->header_detect_entries == NULL) + size_t calloc_size = safe_multiply_size_t(header_detect_io_config->header_detect_entry_count, sizeof(INTERNAL_HEADER_DETECT_ENTRY)); + if (calloc_size == SIZE_MAX || + (result->header_detect_entries = (INTERNAL_HEADER_DETECT_ENTRY*)calloc(1, calloc_size)) == NULL) { + LogError("Could not allocate header_detect_entries, size:%zu", calloc_size); free(result); result = NULL; } diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/link.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/link.c index 7b0d0e0d6..838282d54 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/link.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/link.c @@ -16,8 +16,10 @@ #include "azure_uamqp_c/amqp_definitions.h" #include "azure_uamqp_c/amqp_frame_codec.h" #include "azure_uamqp_c/async_operation.h" +#include "azure_c_shared_utility/safe_math.h" #define DEFAULT_LINK_CREDIT 10000 +#define RECEIVER_MIN_LINK_CREDIT 1 typedef struct DELIVERY_INSTANCE_TAG { @@ -57,7 +59,7 @@ typedef struct LINK_INSTANCE_TAG sequence_no initial_delivery_count; uint64_t max_message_size; uint64_t peer_max_message_size; - int32_t current_link_credit; + uint32_t current_link_credit; uint32_t max_link_credit; uint32_t available; fields attach_properties; @@ -390,7 +392,7 @@ static void link_frame_received(void* context, AMQP_VALUE performative, uint32_t } else if (is_flow_type_by_descriptor(descriptor)) { - FLOW_HANDLE flow_handle; + FLOW_HANDLE flow_handle=NULL; if (amqpvalue_get_flow(performative, &flow_handle) != 0) { LogError("Cannot get flow performative"); @@ -423,9 +425,8 @@ static void link_frame_received(void* context, AMQP_VALUE performative, uint32_t } } } + flow_destroy(flow_handle); } - - flow_destroy(flow_handle); } else if (is_transfer_type_by_descriptor(descriptor)) { @@ -442,6 +443,12 @@ static void link_frame_received(void* context, AMQP_VALUE performative, uint32_t bool more; bool is_error; + if (link_instance->current_link_credit <= RECEIVER_MIN_LINK_CREDIT) + { + link_instance->current_link_credit = link_instance->max_link_credit; + send_flow(link_instance); + } + more = false; /* Attempt to get more flag, default to false */ (void)transfer_get_more(transfer_handle, &more); @@ -462,10 +469,13 @@ static void link_frame_received(void* context, AMQP_VALUE performative, uint32_t /* If this is a continuation transfer or if this is the first chunk of a multi frame transfer */ if ((link_instance->received_payload_size > 0) || more) { - unsigned char* new_received_payload = (unsigned char*)realloc(link_instance->received_payload, (size_t)link_instance->received_payload_size + payload_size); - if (new_received_payload == NULL) + unsigned char* new_received_payload;; + + size_t realloc_size = safe_add_size_t((size_t)link_instance->received_payload_size, payload_size); + if (realloc_size == SIZE_MAX || + (new_received_payload = (unsigned char*)realloc(link_instance->received_payload, realloc_size)) == NULL) { - LogError("Could not allocate memory for the received payload"); + LogError("Could not allocate memory for the received payload, size:%zu", realloc_size); } else { @@ -1640,26 +1650,24 @@ ASYNC_OPERATION_HANDLE link_transfer_async(LINK_HANDLE link, message_format mess default: case SESSION_SEND_TRANSFER_ERROR: LogError("Failed session send transfer"); - if (singlylinkedlist_remove(link->pending_deliveries, delivery_instance_list_item) != 0) + if (singlylinkedlist_remove(link->pending_deliveries, delivery_instance_list_item) == 0) { - LogError("Error removing pending delivery from the list"); + async_operation_destroy(result); } *link_transfer_error = LINK_TRANSFER_ERROR; - async_operation_destroy(result); result = NULL; break; case SESSION_SEND_TRANSFER_BUSY: /* Ensure we remove from list again since sender will attempt to transfer again on flow on */ LogError("Failed session send transfer"); - if (singlylinkedlist_remove(link->pending_deliveries, delivery_instance_list_item) != 0) + if (singlylinkedlist_remove(link->pending_deliveries, delivery_instance_list_item) == 0) { - LogError("Error removing pending delivery from the list"); + async_operation_destroy(result); } *link_transfer_error = LINK_TRANSFER_BUSY; - async_operation_destroy(result); result = NULL; break; 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 0bda7041a..8e9bce06c 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 @@ -10,6 +10,7 @@ #include "azure_uamqp_c/amqp_definitions.h" #include "azure_uamqp_c/message.h" #include "azure_uamqp_c/amqpvalue.h" +#include "azure_c_shared_utility/safe_math.h" typedef struct BODY_AMQP_DATA_TAG { @@ -245,78 +246,108 @@ 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)); + + if (calloc_size == SIZE_MAX) + { + LogError("Invalid size for body_amqp_data_items"); + message_destroy(result); + result = NULL; + } + else + { + result->body_amqp_data_items = (BODY_AMQP_DATA*)calloc(1, calloc_size); - 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) { - /* 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; + /* 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 { - for (i = 0; i < source_message->body_amqp_data_count; i++) - { - result->body_amqp_data_items[i].body_data_section_length = source_message->body_amqp_data_items[i].body_data_section_length; + 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) - { - 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); - } - } + for (i = 0; i < source_message->body_amqp_data_count; i++) + { + result->body_amqp_data_items[i].body_data_section_length = source_message->body_amqp_data_items[i].body_data_section_length; - result->body_amqp_data_count = i; - if (i < source_message->body_amqp_data_count) + /* 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) { - /* 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; + 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); + } + } + + 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 (calloc_size == SIZE_MAX) + { + LogError("Invalid size for body_amqp_sequence_items"); + message_destroy(result); + result = NULL; + } + else + { + 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; + /* 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 { - 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; - } - } + size_t i; - result->body_amqp_sequence_count = i; - if (i < source_message->body_amqp_sequence_count) + 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) { - /* 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; + 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)) @@ -1035,46 +1066,61 @@ 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)); + 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) + { + LogError("Invalid size for new_body_amqp_data_items"); + result = MU_FAILURE; + } + 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, realloc_size); if (new_body_amqp_data_items == 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 items"); - result = MU_FAILURE; + /* 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 = new_body_amqp_data_items; + message->body_amqp_data_items = new_body_amqp_data_items; - if (amqp_data.length == 0) + if (amqp_data.length == 0) + { + 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) { - 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 to be added"); + 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[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++; + 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; - } + /* Codes_SRS_MESSAGE_01_087: [ On success it shall return 0. ]*/ + result = 0; } + } + } } } } @@ -1195,7 +1241,7 @@ int message_set_body_amqp_value(MESSAGE_HANDLE message, AMQP_VALUE body_amqp_val /* Only free the previous value when cloning is succesfull */ if (message->body_amqp_value != NULL) { - amqpvalue_destroy(body_amqp_value); + amqpvalue_destroy(message->body_amqp_value); } /* Codes_SRS_MESSAGE_01_101: [ `message_set_body_amqp_value` shall set the contents of body as being the AMQP value indicate by `body_amqp_value`. ]*/ @@ -1268,7 +1314,17 @@ 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)); + 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) + { + LogError("Invalid size for new_body_amqp_sequence_items"); + result = MU_FAILURE; + } + else + { + AMQP_VALUE* new_body_amqp_sequence_items = (AMQP_VALUE*)realloc(message->body_amqp_sequence_items, realloc_size); if (new_body_amqp_sequence_items == NULL) { /* 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. ]*/ @@ -1277,25 +1333,31 @@ int message_add_body_amqp_sequence(MESSAGE_HANDLE message, AMQP_VALUE sequence_l } else { - message->body_amqp_sequence_items = new_body_amqp_sequence_items; + message->body_amqp_sequence_items = new_body_amqp_sequence_items; - /* 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_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; - } + /* Codes_SRS_MESSAGE_01_111: [ On success it shall return 0. ]*/ + result = 0; + } + } } } } 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 c387254e4..674116a7c 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 @@ -15,6 +15,7 @@ #include "azure_uamqp_c/amqpvalue_to_string.h" #include "azure_uamqp_c/async_operation.h" #include "azure_uamqp_c/amqp_definitions.h" +#include "azure_c_shared_utility/safe_math.h" typedef enum MESSAGE_SEND_STATE_TAG { @@ -75,8 +76,9 @@ static void remove_pending_message_by_index(MESSAGE_SENDER_HANDLE message_sender if (message_sender->message_count > 0) { - new_messages = (ASYNC_OPERATION_HANDLE*)realloc(message_sender->messages, sizeof(ASYNC_OPERATION_HANDLE) * (message_sender->message_count)); - if (new_messages != NULL) + size_t realloc_size = safe_multiply_size_t(sizeof(ASYNC_OPERATION_HANDLE), (message_sender->message_count)); + if (realloc_size != SIZE_MAX && + (new_messages = (ASYNC_OPERATION_HANDLE*)realloc(message_sender->messages, realloc_size)) != NULL) { message_sender->messages = new_messages; } @@ -106,12 +108,12 @@ static void on_delivery_settled(void* context, delivery_number delivery_no, LINK { ASYNC_OPERATION_HANDLE pending_send = (ASYNC_OPERATION_HANDLE)context; MESSAGE_WITH_CALLBACK* message_with_callback = GET_ASYNC_OPERATION_CONTEXT(MESSAGE_WITH_CALLBACK, pending_send); - MESSAGE_SENDER_INSTANCE* message_sender = (MESSAGE_SENDER_INSTANCE*)message_with_callback->message_sender; (void)delivery_no; if (message_with_callback != NULL && message_with_callback->on_message_send_complete != NULL) { + MESSAGE_SENDER_INSTANCE *message_sender = (MESSAGE_SENDER_INSTANCE *)message_with_callback->message_sender; switch (reason) { case LINK_DELIVERY_SETTLE_REASON_DISPOSITION_RECEIVED: @@ -1072,11 +1074,14 @@ ASYNC_OPERATION_HANDLE messagesender_send_async(MESSAGE_SENDER_HANDLE message_se } else { + ASYNC_OPERATION_HANDLE* new_messages; MESSAGE_WITH_CALLBACK* message_with_callback = GET_ASYNC_OPERATION_CONTEXT(MESSAGE_WITH_CALLBACK, result); - ASYNC_OPERATION_HANDLE* new_messages = (ASYNC_OPERATION_HANDLE*)realloc(message_sender->messages, sizeof(ASYNC_OPERATION_HANDLE) * (message_sender->message_count + 1)); - if (new_messages == NULL) + 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 (realloc_size == SIZE_MAX || + (new_messages = (ASYNC_OPERATION_HANDLE*)realloc(message_sender->messages, realloc_size)) == NULL) { - LogError("Failed allocating memory for pending sends"); + LogError("Failed allocating memory for pending sends, size:%zu", realloc_size); async_operation_destroy(result); result = NULL; } diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/sasl_frame_codec.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/sasl_frame_codec.c index efa50f800..5a22f3947 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/sasl_frame_codec.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/sasl_frame_codec.c @@ -12,11 +12,13 @@ #include "azure_uamqp_c/frame_codec.h" #include "azure_uamqp_c/amqpvalue.h" #include "azure_uamqp_c/amqp_definitions.h" +#include "azure_c_shared_utility/safe_math.h" /* Requirements implemented by design or by other modules */ /* Codes_SRS_SASL_FRAME_CODEC_01_011: [A SASL frame has a type code of 0x01.] */ /* Codes_SRS_SASL_FRAME_CODEC_01_016: [The maximum size of a SASL frame is defined by MIN-MAX-FRAME-SIZE.] */ - +/* Codes_SRS_SASL_FRAME_CODEC_01_017: [The minimum size of a SASL frame is defined by MIN_FRAME_SIZE (1 byte).] */ +#define MIN_FRAME_SIZE 1 #define MIX_MAX_FRAME_SIZE 512 typedef enum SASL_FRAME_DECODE_STATE_TAG @@ -82,7 +84,9 @@ static void frame_received(void* context, const unsigned char* type_specific, ui /* Codes_SRS_SASL_FRAME_CODEC_01_007: [The extended header is ignored.] */ /* Codes_SRS_SASL_FRAME_CODEC_01_008: [The maximum size of a SASL frame is defined by MIN-MAX-FRAME-SIZE.] */ - if ((type_specific_size + frame_body_size + 6 > MIX_MAX_FRAME_SIZE) || + size_t size = safe_add_size_t(type_specific_size, frame_body_size); + size = safe_add_size_t(size, 6); + if ((size > MIX_MAX_FRAME_SIZE) || /* Codes_SRS_SASL_FRAME_CODEC_01_010: [Receipt of an empty frame is an irrecoverable error.] */ (frame_body_size == 0)) { @@ -277,11 +281,12 @@ int sasl_frame_codec_encode_frame(SASL_FRAME_CODEC_HANDLE sasl_frame_codec, AMQP LogError("Cannot get SASL frame encoded size"); result = MU_FAILURE; } - /* Codes_SRS_SASL_FRAME_CODEC_01_016: [The maximum size of a SASL frame is defined by MIN-MAX-FRAME-SIZE.] */ - else if (encoded_size > MIX_MAX_FRAME_SIZE - 8) + /* Codes_SRS_SASL_FRAME_CODEC_01_016: [The maximum size of a SASL frame is defined by MIN-MAX-FRAME-SIZE.] */ + /* Codes_SRS_SASL_FRAME_CODEC_01_017: [The minimum size of a SASL frame is defined by MIN_FRAME_SIZE (1 byte).] */ + else if (encoded_size < MIN_FRAME_SIZE || encoded_size > (MIX_MAX_FRAME_SIZE - 8)) { /* Codes_SRS_SASL_FRAME_CODEC_01_034: [If any error occurs during encoding, sasl_frame_codec_encode_frame shall fail and return a non-zero value.] */ - LogError("SASL frame encoded size too big"); + LogError("SASL frame encoded size out of bounds (%u)", (unsigned int)encoded_size); result = MU_FAILURE; } else diff --git a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/session.c b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/session.c index ab1d0802f..c087041b0 100644 --- a/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/session.c +++ b/sdk/core/azure-core-amqp/vendor/azure-uamqp-c/src/session.c @@ -96,8 +96,9 @@ static void remove_link_endpoint(LINK_ENDPOINT_HANDLE link_endpoint) } else { - new_endpoints = (LINK_ENDPOINT_INSTANCE**)realloc(session_instance->link_endpoints, sizeof(LINK_ENDPOINT_INSTANCE*) * session_instance->link_endpoint_count); - if (new_endpoints != NULL) + size_t realloc_size = safe_multiply_size_t(sizeof(LINK_ENDPOINT_INSTANCE*), session_instance->link_endpoint_count); + if (realloc_size != SIZE_MAX && + (new_endpoints = (LINK_ENDPOINT_INSTANCE**)realloc(session_instance->link_endpoints, realloc_size)) != NULL) { session_instance->link_endpoints = new_endpoints; } @@ -1227,10 +1228,13 @@ LINK_ENDPOINT_HANDLE session_create_link_endpoint(SESSION_HANDLE session, const (void)memcpy(result->name, name, name_length + 1); result->session = session; - new_link_endpoints = (LINK_ENDPOINT_INSTANCE**)realloc(session_instance->link_endpoints, sizeof(LINK_ENDPOINT_INSTANCE*) * ((size_t)session_instance->link_endpoint_count + 1)); - if (new_link_endpoints == NULL) + size_t realloc_size = safe_add_size_t((size_t)session_instance->link_endpoint_count, 1); + realloc_size = safe_multiply_size_t(realloc_size, sizeof(LINK_ENDPOINT_INSTANCE)); + if (realloc_size == SIZE_MAX || + (new_link_endpoints = (LINK_ENDPOINT_INSTANCE**)realloc(session_instance->link_endpoints, realloc_size)) == NULL) { /* Codes_S_R_S_SESSION_01_045: [If allocating memory for the link endpoint fails, session_create_link_endpoint shall fail and return NULL.] */ + LogError("Cannot realloc new_link_endpoints, size:%zu", realloc_size); free(result->name); free(result); result = NULL;