Skip to content

Commit e2cc0b4

Browse files
ericwolzewertons
ericwolz
and
ewertons
authored
memcheck for C SDK (#2583)
* malloc size check * fix unit test * PR feedback * Update iothub_client/src/iothubtransport_amqp_messenger.c Co-authored-by: Ewerton Scaboro da Silva <ewertons@microsoft.com> * Update iothub_client/src/iothubtransport_amqp_telemetry_messenger.c Co-authored-by: Ewerton Scaboro da Silva <ewertons@microsoft.com> * Update iothub_client/src/iothubtransport_amqp_twin_messenger.c Co-authored-by: Ewerton Scaboro da Silva <ewertons@microsoft.com> * Update serializer/inc/serializer_devicetwin.h Co-authored-by: Ewerton Scaboro da Silva <ewertons@microsoft.com> --------- Co-authored-by: Ewerton Scaboro da Silva <ewertons@microsoft.com>
1 parent 9d67f5a commit e2cc0b4

24 files changed

Lines changed: 346 additions & 178 deletions

‎iothub_client/src/iothub_client_authorization.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "azure_c_shared_utility/shared_util_options.h"
1414
#include "azure_c_shared_utility/azure_base64.h"
1515
#include "azure_c_shared_utility/buffer_.h"
16+
#include "azure_c_shared_utility/safe_math.h"
1617

1718
#ifdef USE_PROV_MODULE
1819
#include "azure_prov_client/internal/iothub_auth_client.h"
@@ -572,9 +573,12 @@ static char* read_ca_certificate_from_file(const char* certificate_file_name)
572573
{
573574
rewind(file_stream);
574575

575-
if ((result = calloc(1, file_size + 1)) == NULL)
576+
size_t calloc_size = safe_add_size_t(file_size, 1);
577+
if (calloc_size == SIZE_MAX ||
578+
(result = calloc(1, calloc_size)) == NULL)
576579
{
577-
LogError("Cannot allocate %lu bytes", (unsigned long)file_size);
580+
LogError("Cannot allocate %zu bytes", calloc_size);
581+
result = NULL;
578582
}
579583
else if ((fread(result, 1, file_size, file_stream) == 0) || (ferror(file_stream) != 0))
580584
{

‎iothub_client/src/iothub_client_core_ll.c‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "azure_c_shared_utility/singlylinkedlist.h"
1818
#include "azure_c_shared_utility/shared_util_options.h"
1919
#include "azure_c_shared_utility/agenttime.h"
20+
#include "azure_c_shared_utility/safe_math.h"
2021

2122
#include "iothub_client_core_ll.h"
2223
#include "iothub_client_options.h"
@@ -732,11 +733,14 @@ IOTHUB_CLIENT_RESULT IoTHubClientCore_LL_ParseMethodToCommand(const char* method
732733
}
733734
else
734735
{
735-
size_t component_name_length = componentSplit - method_name;
736-
if ((*component_name = malloc(component_name_length + 1)) == NULL)
736+
size_t component_name_length = safe_subtract_size_t(componentSplit, method_name);
737+
size_t malloc_size = safe_add_size_t(component_name_length, 1);
738+
if (malloc_size == SIZE_MAX ||
739+
(*component_name = malloc(malloc_size)) == NULL)
737740
{
738-
LogError("Cannot allocate component name");
741+
LogError("Cannot allocate component name, size:%zu", malloc_size);
739742
result = IOTHUB_CLIENT_ERROR;
743+
*component_name = NULL;
740744
}
741745
else
742746
{
@@ -975,24 +979,27 @@ static IOTHUB_CLIENT_CORE_LL_HANDLE_DATA* initialize_iothub_client(const IOTHUB_
975979
else
976980
{
977981
size_t suffix_len = strlen(whereIsDot);
978-
IoTHubName = (char*)malloc(whereIsDot - hostname + 1);
979-
if (IoTHubName == NULL)
982+
size_t calloc_size_hub_name = safe_add_size_t(safe_subtract_size_t(whereIsDot, hostname), 1);
983+
size_t malloc_size_hub_suffix = safe_add_size_t(suffix_len, 1);
984+
if (calloc_size_hub_name == SIZE_MAX ||
985+
(IoTHubName = (char*)calloc(1, calloc_size_hub_name)) == NULL)
980986
{
981-
LogError("unable to malloc");
987+
LogError("unable to calloc, size:%zu", calloc_size_hub_name);
982988
IoTHubClient_Auth_Destroy(result->authorization_module);
983989
free(result);
984990
result = NULL;
985991
}
986-
else if ((IoTHubSuffix = (char*)malloc(suffix_len + 1)) == NULL)
992+
else if (malloc_size_hub_suffix == SIZE_MAX ||
993+
(IoTHubSuffix = (char*)malloc(malloc_size_hub_suffix)) == NULL)
987994
{
988995
LogError("unable to malloc");
989996
IoTHubClient_Auth_Destroy(result->authorization_module);
997+
free(IoTHubName);
990998
free(result);
991999
result = NULL;
9921000
}
9931001
else
9941002
{
995-
memset(IoTHubName, 0, whereIsDot - hostname + 1);
9961003
(void)memcpy(IoTHubName, hostname, whereIsDot - hostname);
9971004
(void)strcpy(IoTHubSuffix, whereIsDot+1);
9981005

@@ -1270,34 +1277,34 @@ IOTHUB_CLIENT_CORE_LL_HANDLE IoTHubClientCore_LL_CreateFromDeviceAuth(const char
12701277
{
12711278
if (*iterator == '.')
12721279
{
1273-
size_t length = iterator - initial;
1274-
iothub_name = (char*)malloc(length + 1);
1275-
if (iothub_name != NULL)
1280+
size_t length = safe_subtract_size_t(iterator, initial);
1281+
size_t calloc_size = safe_add_size_t(length, 1);
1282+
if (calloc_size != SIZE_MAX &&
1283+
(iothub_name = (char*)calloc(1, calloc_size)) != NULL)
12761284
{
1277-
memset(iothub_name, 0, length + 1);
12781285
memcpy(iothub_name, initial, length);
12791286
config->iotHubName = iothub_name;
12801287

1281-
length = strlen(initial) - length - 1;
1282-
iothub_suffix = (char*)malloc(length + 1);
1283-
if (iothub_suffix != NULL)
1288+
length = safe_subtract_size_t(safe_subtract_size_t(strlen(initial), length), 1);
1289+
calloc_size = safe_add_size_t(length, 1);
1290+
if (calloc_size != SIZE_MAX &&
1291+
(iothub_suffix = (char*)calloc(1, calloc_size)) != NULL)
12841292
{
1285-
memset(iothub_suffix, 0, length + 1);
12861293
memcpy(iothub_suffix, iterator + 1, length);
12871294
config->iotHubSuffix = iothub_suffix;
12881295
break;
12891296
}
12901297
else
12911298
{
1292-
LogError("Failed to allocate iothub suffix");
1299+
LogError("Failed to allocate iothub suffix, size:%zu", calloc_size);
12931300
free(iothub_name);
12941301
iothub_name = NULL;
12951302
result = NULL;
12961303
}
12971304
}
12981305
else
12991306
{
1300-
LogError("Failed to allocate iothub name");
1307+
LogError("Failed to allocate iothub name, size:%zu", calloc_size);
13011308
result = NULL;
13021309
}
13031310
}

‎iothub_client/src/iothub_client_ll_uploadtoblob.c‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "azure_c_shared_utility/shared_util_options.h"
1616
#include "azure_c_shared_utility/urlencode.h"
1717
#include "azure_c_shared_utility/crt_abstractions.h"
18+
#include "azure_c_shared_utility/safe_math.h"
1819

1920
#include "iothub_client_core_ll.h"
2021
#include "iothub_client_options.h"
@@ -766,22 +767,25 @@ static int parseAzureBlobSasUri(const char* sasUri, char** blobStorageHostname,
766767
}
767768
else
768769
{
769-
size_t hostnameSize = relativePathBegin - hostnameBegin;
770-
771-
if ((hostname = malloc(hostnameSize + 1)) == NULL)
770+
size_t hostnameSize = safe_subtract_size_t(relativePathBegin, hostnameBegin);
771+
size_t malloc_size = safe_add_size_t(hostnameSize, 1);
772+
if (malloc_size == SIZE_MAX ||
773+
(hostname = malloc(malloc_size)) == NULL)
772774
{
773-
LogError("failed to allocate memory for blob storage hostname");
775+
LogError("failed to allocate memory for blob storage hostname, size:%zu", malloc_size);
774776
result = MU_FAILURE;
775777
}
776778
else
777779
{
778780
(void)memcpy(hostname, hostnameBegin, hostnameSize);
779781
hostname[hostnameSize] = '\0';
780-
size_t relativePathSize = sasUriLength - (relativePathBegin - sasUri);
782+
size_t relativePathSize = safe_subtract_size_t(sasUriLength, safe_subtract_size_t(relativePathBegin, sasUri));
783+
malloc_size = safe_add_size_t(relativePathSize, 1);
781784

782-
if ((relativePath = malloc(relativePathSize + 1)) == NULL)
785+
if (malloc_size == SIZE_MAX ||
786+
(relativePath = malloc(malloc_size)) == NULL)
783787
{
784-
LogError("failed to allocate memory for blob storage relative path");
788+
LogError("failed to allocate memory for blob storage relative path, size:%zu", malloc_size);
785789
free(hostname);
786790
result = MU_FAILURE;
787791
}
@@ -824,10 +828,13 @@ IOTHUB_CLIENT_LL_UPLOADTOBLOB_HANDLE IoTHubClient_LL_UploadToBlob_Create(const I
824828

825829
size_t iotHubNameLength = strlen(config->iotHubName);
826830
size_t iotHubSuffixLength = strlen(config->iotHubSuffix);
827-
upload_data->hostname = malloc(iotHubNameLength + 1 + iotHubSuffixLength + 1); /*first +1 is because "." the second +1 is because \0*/
828-
if (upload_data->hostname == NULL)
831+
832+
/*first +1 is because "." the second +1 is because \0*/
833+
size_t malloc_size = safe_add_size_t(safe_add_size_t(safe_add_size_t(iotHubNameLength, 1), iotHubSuffixLength), 1);
834+
if (malloc_size == SIZE_MAX ||
835+
(upload_data->hostname = malloc(malloc_size)) == NULL)
829836
{
830-
LogError("Failed malloc allocation");
837+
LogError("Failed malloc allocation, size:%zu", malloc_size);
831838
free(upload_data);
832839
upload_data = NULL;
833840
}

‎iothub_client/src/iothubtransport_amqp_connection.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "azure_c_shared_utility/gballoc.h"
99
#include "azure_c_shared_utility/strings.h"
1010
#include "azure_c_shared_utility/uniqueid.h"
11+
#include "azure_c_shared_utility/safe_math.h"
1112
#include "azure_uamqp_c/sasl_mechanism.h"
1213
#include "azure_uamqp_c/saslclientio.h"
1314
#include "azure_uamqp_c/sasl_mssbcbs.h"
@@ -165,15 +166,15 @@ static int create_connection_handle(AMQP_CONNECTION_INSTANCE* instance)
165166
connection_io_transport = instance->underlying_io_transport;
166167
}
167168

168-
if ((unique_container_id = (char*)malloc(sizeof(char) * DEFAULT_UNIQUE_ID_LENGTH + 1)) == NULL)
169+
size_t calloc_size = safe_multiply_size_t(sizeof(char), safe_add_size_t(DEFAULT_UNIQUE_ID_LENGTH, 1));
170+
if (calloc_size == SIZE_MAX ||
171+
(unique_container_id = (char*)calloc(1, calloc_size)) == NULL)
169172
{
170173
result = __LINE__;
171-
LogError("Failed creating the AMQP connection (failed creating unique ID container)");
174+
LogError("Failed creating the AMQP connection (failed creating unique ID container), size:%zu", calloc_size);
172175
}
173176
else
174177
{
175-
memset(unique_container_id, 0, sizeof(char) * DEFAULT_UNIQUE_ID_LENGTH + 1);
176-
177178
if (UniqueId_Generate(unique_container_id, DEFAULT_UNIQUE_ID_LENGTH) != UNIQUEID_OK)
178179
{
179180
result = MU_FAILURE;

‎iothub_client/src/iothubtransport_amqp_messenger.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "azure_c_shared_utility/agenttime.h"
1010
#include "azure_c_shared_utility/xlogging.h"
1111
#include "azure_c_shared_utility/uniqueid.h"
12+
#include "azure_c_shared_utility/safe_math.h"
1213
#include "azure_uamqp_c/link.h"
1314
#include "azure_uamqp_c/messaging.h"
1415
#include "azure_uamqp_c/message_sender.h"
@@ -337,15 +338,15 @@ static STRING_HANDLE create_link_name(role link_role, const char* device_id)
337338
char* unique_id;
338339
STRING_HANDLE result;
339340

340-
if ((unique_id = (char*)malloc(sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1)) == NULL)
341+
size_t calloc_size = safe_multiply_size_t(sizeof(char), safe_add_size_t(UNIQUE_ID_BUFFER_SIZE, 1));
342+
if (calloc_size == SIZE_MAX ||
343+
(unique_id = (char*)calloc(1, calloc_size)) == NULL)
341344
{
342-
LogError("Failed generating an unique tag (malloc failed)");
345+
LogError("Failed generating an unique tag (calloc failed), size:%zu", calloc_size);
343346
result = NULL;
344347
}
345348
else
346349
{
347-
memset(unique_id, 0, sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1);
348-
349350
if (UniqueId_Generate(unique_id, UNIQUE_ID_BUFFER_SIZE) != UNIQUEID_OK)
350351
{
351352
LogError("Failed generating an unique tag (UniqueId_Generate failed)");

‎iothub_client/src/iothubtransport_amqp_telemetry_messenger.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "azure_c_shared_utility/xlogging.h"
1212
#include "azure_c_shared_utility/uniqueid.h"
1313
#include "azure_c_shared_utility/singlylinkedlist.h"
14+
#include "azure_c_shared_utility/safe_math.h"
1415
#include "azure_uamqp_c/link.h"
1516
#include "azure_uamqp_c/messaging.h"
1617
#include "azure_uamqp_c/message_sender.h"
@@ -269,14 +270,14 @@ static STRING_HANDLE create_link_name(const char* prefix, const char* infix)
269270
char* unique_id;
270271
STRING_HANDLE tag = NULL;
271272

272-
if ((unique_id = (char*)malloc(sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1)) == NULL)
273+
size_t calloc_size = safe_multiply_size_t(sizeof(char), safe_add_size_t(UNIQUE_ID_BUFFER_SIZE, 1));
274+
if (calloc_size == SIZE_MAX ||
275+
(unique_id = (char*)calloc(1, calloc_size)) == NULL)
273276
{
274-
LogError("Failed generating an unique tag (malloc failed)");
277+
LogError("Failed generating an unique tag (calloc failed), size:%zu", calloc_size);
275278
}
276279
else
277280
{
278-
memset(unique_id, 0, sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1);
279-
280281
if (UniqueId_Generate(unique_id, UNIQUE_ID_BUFFER_SIZE) != UNIQUEID_OK)
281282
{
282283
LogError("Failed generating an unique tag (UniqueId_Generate failed)");

‎iothub_client/src/iothubtransport_amqp_twin_messenger.c‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "azure_c_shared_utility/xlogging.h"
1212
#include "azure_c_shared_utility/uniqueid.h"
1313
#include "azure_c_shared_utility/singlylinkedlist.h"
14+
#include "azure_c_shared_utility/safe_math.h"
1415
#include "azure_uamqp_c/amqp_definitions_fields.h"
1516
#include "azure_uamqp_c/messaging.h"
1617
#include "internal/iothub_client_private.h"
@@ -341,14 +342,15 @@ static char* generate_unique_id(void)
341342
{
342343
char* result;
343344

344-
if ((result = (char*)malloc(sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1)) == NULL)
345+
size_t calloc_size = safe_multiply_size_t(sizeof(char), safe_add_size_t(UNIQUE_ID_BUFFER_SIZE, 1));
346+
if (calloc_size == SIZE_MAX ||
347+
(result = (char*)calloc(1, calloc_size)) == NULL)
345348
{
346-
LogError("Failed generating an unique tag (malloc failed)");
349+
LogError("Failed generating an unique tag (calloc failed), size:%zu", calloc_size);
350+
result = NULL;
347351
}
348352
else
349353
{
350-
memset(result, 0, sizeof(char) * UNIQUE_ID_BUFFER_SIZE + 1);
351-
352354
if (UniqueId_Generate(result, UNIQUE_ID_BUFFER_SIZE) != UNIQUEID_OK)
353355
{
354356
LogError("Failed generating an unique tag (UniqueId_Generate failed)");
@@ -372,9 +374,14 @@ static char* generate_twin_correlation_id(void)
372374
}
373375
else
374376
{
375-
if ((result = (char*)malloc(strlen(TWIN_CORRELATION_ID_PROPERTY_FORMAT) + strlen(unique_id) + 1)) == NULL)
377+
size_t malloc_size = strlen(TWIN_CORRELATION_ID_PROPERTY_FORMAT);
378+
malloc_size = safe_add_size_t(malloc_size, strlen(unique_id));
379+
malloc_size = safe_add_size_t(malloc_size, 1);
380+
381+
if (malloc_size == SIZE_MAX ||
382+
(result = (char*)malloc(malloc_size)) == NULL)
376383
{
377-
LogError("Failed allocating correlation-id");
384+
LogError("Failed allocating correlation-id, size:%zu", malloc_size);
378385
result = NULL;
379386
}
380387
else

‎iothub_client/src/iothubtransport_mqtt_common.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "azure_c_shared_utility/tickcounter.h"
1717
#include "azure_c_shared_utility/tlsio.h"
1818
#include "azure_c_shared_utility/platform.h"
19+
#include "azure_c_shared_utility/safe_math.h"
1920
#include "azure_c_shared_utility/string_tokenizer.h"
2021
#include "azure_c_shared_utility/shared_util_options.h"
2122
#include "azure_c_shared_utility/urlencode.h"
@@ -1438,10 +1439,12 @@ static const char* addInputNamePropertyToMsg(IOTHUB_MESSAGE_HANDLE iotHubMessage
14381439
}
14391440
else
14401441
{
1441-
size_t inputNameLength = inputNameEnd - inputNameStart;
1442-
if ((inputNameCopy = malloc(inputNameLength + 1)) == NULL)
1442+
size_t inputNameLength = safe_subtract_size_t(inputNameEnd, inputNameStart);
1443+
size_t malloc_size = safe_add_size_t(inputNameLength, 1);
1444+
if (malloc_size == SIZE_MAX ||
1445+
(inputNameCopy = malloc(malloc_size)) == NULL)
14431446
{
1444-
LogError("Cannot allocate input name");
1447+
LogError("Cannot allocate input name, size:%zu", malloc_size);
14451448
result = NULL;
14461449
}
14471450
else
@@ -1641,9 +1644,11 @@ static int addApplicationPropertyToMessage(MAP_HANDLE propertyMap, const char* p
16411644
// We need to make a copy of propertyName at this point; we don't own the buffer it's part of
16421645
// so we can't just sneak a temporary '\0' in there. We don't need to make a copy of propertyValue
16431646
// since its part of an otherwise null terminated string.
1644-
if ((propertyNameCopy = (char*)malloc(propertyNameLength + 1)) == NULL)
1647+
size_t malloc_size = safe_add_size_t(propertyNameLength, 1);
1648+
if (malloc_size == SIZE_MAX ||
1649+
(propertyNameCopy = (char*)malloc(malloc_size)) == NULL)
16451650
{
1646-
LogError("Failed allocating property information");
1651+
LogError("Failed allocating property information, size:%zu", malloc_size);
16471652
result = MU_FAILURE;
16481653
}
16491654
else

‎iothub_client/src/iothubtransportamqp_methods.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "azure_c_shared_utility/crt_abstractions.h"
99
#include "azure_c_shared_utility/xlogging.h"
1010
#include "azure_c_shared_utility/strings.h"
11+
#include "azure_c_shared_utility/safe_math.h"
1112
#include "azure_uamqp_c/link.h"
1213
#include "azure_uamqp_c/messaging.h"
1314
#include "azure_uamqp_c/message_receiver.h"
@@ -84,11 +85,17 @@ static void remove_tracked_handle(IOTHUBTRANSPORT_AMQP_METHODS* amqp_methods_han
8485
}
8586
else
8687
{
87-
IOTHUBTRANSPORT_AMQP_METHOD_HANDLE* new_handles = (IOTHUBTRANSPORT_AMQP_METHOD_HANDLE*)realloc(amqp_methods_handle->method_request_handles, amqp_methods_handle->method_request_handle_count * sizeof(IOTHUBTRANSPORT_AMQP_METHOD_HANDLE));
88-
if (new_handles != NULL)
88+
IOTHUBTRANSPORT_AMQP_METHOD_HANDLE* new_handles;
89+
size_t realloc_size = safe_multiply_size_t(amqp_methods_handle->method_request_handle_count, sizeof(IOTHUBTRANSPORT_AMQP_METHOD_HANDLE));
90+
if (realloc_size != SIZE_MAX && (
91+
new_handles = (IOTHUBTRANSPORT_AMQP_METHOD_HANDLE*)realloc(amqp_methods_handle->method_request_handles, realloc_size)) != NULL)
8992
{
9093
amqp_methods_handle->method_request_handles = new_handles;
9194
}
95+
else
96+
{
97+
LogError("realloc error, size:%zu", realloc_size);
98+
}
9299
}
93100
}
94101

@@ -281,11 +288,12 @@ static AMQP_VALUE on_message_received(const void* context, MESSAGE_HANDLE messag
281288
else
282289
{
283290
IOTHUBTRANSPORT_AMQP_METHOD_HANDLE* new_handles;
284-
new_handles = (IOTHUBTRANSPORT_AMQP_METHOD_HANDLE*)realloc(amqp_methods_handle->method_request_handles, (amqp_methods_handle->method_request_handle_count + 1) * sizeof(IOTHUBTRANSPORT_AMQP_METHOD_HANDLE));
285-
if (new_handles == NULL)
291+
size_t realloc_size = safe_multiply_size_t(safe_add_size_t(amqp_methods_handle->method_request_handle_count, 1), sizeof(IOTHUBTRANSPORT_AMQP_METHOD_HANDLE));
292+
if (realloc_size == SIZE_MAX ||
293+
(new_handles = (IOTHUBTRANSPORT_AMQP_METHOD_HANDLE*)realloc(amqp_methods_handle->method_request_handles, realloc_size)) == NULL)
286294
{
287295
free(method_handle);
288-
LogError("Cannot grow method handles array");
296+
LogError("Cannot grow method handles array, size:%zu", realloc_size);
289297
message_outcome = MESSAGE_OUTCOME_RELEASED;
290298
}
291299
else

0 commit comments

Comments
 (0)