Skip to content

Commit 15c870e

Browse files
harindhu007harindhu007
committed
Issue 59: Potential Redundant Memory Release in Error Handling
Reason for change: Add a check before free, and fix the Memory free in Error Handling Risks: None Priority: P1 Signed-off-by: Harikrishna Srinivasan <hsrini103@comcast.com>
1 parent cc6d0ea commit 15c870e

9 files changed

+106
-112
lines changed

‎src/sec_adapter_asn1kc.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2021 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -713,7 +713,7 @@ Sec_Result SecAsn1KC_AddAttrBuffer(Sec_Asn1KC* kc, const char* key, void* buf, S
713713
} while (false);
714714

715715
if (result != SEC_RESULT_SUCCESS) {
716-
free(ptr);
716+
SEC_FREE(ptr);
717717
}
718718
return result;
719719
}

‎src/sec_adapter_cipher.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2023 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -964,7 +964,7 @@ Sec_Result SecCipher_ProcessOpaqueWithMap(Sec_CipherHandle* cipherHandle, SEC_BY
964964
out_buffer.context.svp.offset = 0;
965965
out_buffer.context.svp.buffer = get_svp_buffer(cipherHandle->processorHandle, *opaqueBufferHandle);
966966
if (out_buffer.context.svp.buffer == INVALID_HANDLE) {
967-
free(subsample_lengths);
967+
SEC_FREE(subsample_lengths);
968968
SecOpaqueBuffer_Free(*opaqueBufferHandle);
969969
*opaqueBufferHandle = NULL;
970970
*bytesWritten = 0;
@@ -989,7 +989,7 @@ Sec_Result SecCipher_ProcessOpaqueWithMap(Sec_CipherHandle* cipherHandle, SEC_BY
989989
sample.in = &in_buffer;
990990

991991
sa_status status = sa_invoke(cipherHandle->processorHandle, SA_PROCESS_COMMON_ENCRYPTION, (size_t) 1, &sample);
992-
free(subsample_lengths);
992+
SEC_FREE(subsample_lengths);
993993
if (status != SA_STATUS_OK) {
994994
SecOpaqueBuffer_Free(*opaqueBufferHandle);
995995
*opaqueBufferHandle = NULL;
@@ -1058,7 +1058,7 @@ Sec_Result SecCipher_ProcessOpaqueWithMapAndPattern(Sec_CipherHandle* cipherHand
10581058
out_buffer.context.svp.offset = 0;
10591059
out_buffer.context.svp.buffer = get_svp_buffer(cipherHandle->processorHandle, *opaqueBufferHandle);
10601060
if (out_buffer.context.svp.buffer == INVALID_HANDLE) {
1061-
free(subsample_lengths);
1061+
SEC_FREE(subsample_lengths);
10621062
SecOpaqueBuffer_Free(*opaqueBufferHandle);
10631063
*opaqueBufferHandle = NULL;
10641064
*bytesWritten = 0;
@@ -1083,7 +1083,7 @@ Sec_Result SecCipher_ProcessOpaqueWithMapAndPattern(Sec_CipherHandle* cipherHand
10831083
sample.in = &in_buffer;
10841084

10851085
sa_status status = sa_invoke(cipherHandle->processorHandle, SA_PROCESS_COMMON_ENCRYPTION, (size_t) 1, &sample);
1086-
free(subsample_lengths);
1086+
SEC_FREE(subsample_lengths);
10871087
if (status != SA_STATUS_OK) {
10881088
SecOpaqueBuffer_Free(*opaqueBufferHandle);
10891089
*opaqueBufferHandle = NULL;

‎src/sec_adapter_key.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,7 @@ Sec_Result SecKey_ECDHKeyAgreementWithKDF(Sec_KeyHandle* keyHandle, Sec_ECCRawPu
994994

995995
sa_status status = sa_invoke(keyHandle->processorHandle, SA_KEY_EXCHANGE, &shared_secret, &rights,
996996
SA_KEY_EXCHANGE_ALGORITHM_ECDH, keyHandle->key.handle, other_public, (size_t) other_public_length, NULL);
997-
free(other_public);
997+
SEC_FREE(other_public);
998998
CHECK_STATUS(status)
999999

10001000
// Derive the key from the shared secret using a key derivation algorithm.
@@ -2697,21 +2697,21 @@ static bool is_jwt_key_container(SEC_BYTE* key_buffer, SEC_SIZE key_length) {
26972697
SEC_SIZE length;
26982698
SEC_BYTE* header = malloc(header_length);
26992699
Sec_Result result = SecUtils_Base64Decode(header_b64, header_b64_length, header, header_length, &length);
2700-
free(header);
2700+
SEC_FREE(header);
27012701
if (result != SEC_RESULT_SUCCESS)
27022702
return false;
27032703

27042704
SEC_SIZE payload_length = 3 * payload_b64_length / 4;
27052705
SEC_BYTE* payload = malloc(payload_length);
27062706
result = SecUtils_Base64Decode(payload_b64, payload_b64_length, payload, payload_length, &length);
2707-
free(payload);
2707+
SEC_FREE(payload);
27082708
if (result != SEC_RESULT_SUCCESS)
27092709
return false;
27102710

27112711
SEC_SIZE mac_length = 3 * mac_b64_length / 4;
27122712
SEC_BYTE* mac = malloc(mac_length);
27132713
result = SecUtils_Base64Decode(mac_b64, mac_b64_length, mac, mac_length, &length);
2714-
free(mac);
2714+
SEC_FREE(mac);
27152715
if (result != SEC_RESULT_SUCCESS)
27162716
return false;
27172717

‎src/sec_adapter_keyexchange.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2023 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -315,7 +315,7 @@ Sec_Result SecKeyExchange_ComputeSecret(Sec_KeyExchangeHandle* keyExchangeHandle
315315
EVP_PKEY_free(evp_pkey);
316316
if (key_len <= 0) {
317317
SEC_LOG_ERROR("i2d_PUBKEY failed");
318-
free(public_key_bytes);
318+
SEC_FREE(public_key_bytes);
319319
return SEC_RESULT_FAILURE;
320320
}
321321

@@ -350,7 +350,7 @@ Sec_Result SecKeyExchange_ComputeSecret(Sec_KeyExchangeHandle* keyExchangeHandle
350350
sa_key shared_secret;
351351
sa_status status = sa_invoke(keyExchangeHandle->processorHandle, SA_KEY_EXCHANGE, &shared_secret, &rights,
352352
algorithm, *keyExchangeHandle->key, public_key_bytes, (size_t) key_len, NULL);
353-
free(public_key_bytes);
353+
SEC_FREE(public_key_bytes);
354354
CHECK_STATUS(status)
355355

356356
Sec_Key key = {.handle = shared_secret};

‎src/sec_adapter_mac.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2022 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -116,15 +116,15 @@ Sec_Result SecMac_GetInstance(Sec_ProcessorHandle* processorHandle, Sec_MacAlgor
116116
break;
117117

118118
default:
119-
free(newMacHandle);
119+
SEC_FREE(newMacHandle);
120120
return SEC_RESULT_INVALID_PARAMETERS;
121121
}
122122

123123
const Sec_Key* key = get_key(keyHandle);
124124
sa_status status = sa_invoke(processorHandle, SA_CRYPTO_MAC_INIT, &newMacHandle->mac_context, mac_algorithm,
125125
key->handle, parameters);
126126
if (status != SA_STATUS_OK)
127-
free(newMacHandle);
127+
SEC_FREE(newMacHandle);
128128

129129
CHECK_STATUS(status)
130130
*macHandle = newMacHandle;

‎src/sec_adapter_processor.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2023 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -404,7 +404,7 @@ Sec_Result SecProcessor_Release(Sec_ProcessorHandle* processorHandle) {
404404

405405
SEC_FREE(processorHandle->app_dir);
406406
SEC_FREE(processorHandle->global_dir);
407-
free(processorHandle);
407+
SEC_FREE(processorHandle);
408408
return SEC_RESULT_SUCCESS;
409409
}
410410

@@ -429,7 +429,7 @@ void Sec_NativeFree(Sec_ProcessorHandle* processorHandle, void* ptr) {
429429
if (processorHandle == NULL)
430430
return;
431431

432-
free(ptr);
432+
SEC_FREE(ptr);
433433
}
434434

435435
Sec_Result Sec_SetStorageDir(const char* provided_dir, const char* default_dir, char* output_dir) {

‎src/sec_adapter_pubops.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2020-2021 Comcast Cable Communications Management, LLC
2+
* Copyright 2020-2025 Comcast Cable Communications Management, LLC
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -661,7 +661,7 @@ Sec_Result Pubops_ExtractECCPubToPUBKEYDer(Sec_ECCRawPublicKey* eccRawPublicKey,
661661
EC_KEY_free(ec_key);
662662
if (*outLength <= 0) {
663663
SEC_LOG_ERROR("i2d_EC_PUBKEY failed");
664-
free(*out);
664+
SEC_FREE(*out);
665665
return SEC_RESULT_FAILURE;
666666
}
667667

0 commit comments

Comments
 (0)