Fixes for CVE-2024-21646 (#5251)

* Fixes for CVE-2024-21646

* Added note about this fix to CHANGELOG.md
This commit is contained in:
Larry Osterman 2024-01-11 11:33:25 -08:00 committed by GitHub
parent dece333752
commit 23ecf87ecb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 100 additions and 13 deletions

View File

@ -14,6 +14,7 @@
- Fixed several memory leaks.
- AMQP Link Credits now work as expected.
- Integrated the fix for NVD - CVE-2024-21646.
### Other Changes

View File

@ -11,6 +11,7 @@
#include "azure_uamqp_c/amqp_types.h"
#include "azure_uamqp_c/amqpvalue.h"
#include "azure_c_shared_utility/refcount.h"
#include "azure_c_shared_utility/safe_math.h"
// max alloc size 100MB
#define MAX_AMQPVALUE_MALLOC_SIZE_BYTES (100 * 1024 * 1024)
@ -1069,8 +1070,19 @@ AMQP_VALUE amqpvalue_create_string(const char* value)
else
{
size_t length = strlen(value);
size_t malloc_size = length + 1;
result = REFCOUNT_TYPE_CREATE(AMQP_VALUE_DATA);
// If the result of malloc_size is zero it means it had a type overflow (size_t is an unsigned
// type). It is very unlikely but could happen.
if (malloc_size == 0)
{
LogError("Invalid string size exceeded max allocation");
result = NULL;
}
else
{
result = REFCOUNT_TYPE_CREATE(AMQP_VALUE_DATA);
}
if (result == NULL)
{
/* Codes_SRS_AMQPVALUE_01_136: [If allocating the AMQP_VALUE fails then amqpvalue_create_string shall return NULL.] */
@ -1079,7 +1091,7 @@ AMQP_VALUE amqpvalue_create_string(const char* value)
else
{
result->type = AMQP_TYPE_STRING;
result->value.string_value.chars = (char*)malloc(length + 1);
result->value.string_value.chars = (char*)malloc(malloc_size);
if (result->value.string_value.chars == NULL)
{
/* Codes_SRS_AMQPVALUE_01_136: [If allocating the AMQP_VALUE fails then amqpvalue_create_string shall return NULL.] */
@ -1089,7 +1101,7 @@ AMQP_VALUE amqpvalue_create_string(const char* value)
}
else
{
(void)memcpy(result->value.string_value.chars, value, length + 1);
(void)memcpy(result->value.string_value.chars, value, malloc_size);
}
}
}
@ -1145,7 +1157,7 @@ AMQP_VALUE amqpvalue_create_symbol(const char* value)
else
{
size_t length = strlen(value);
if (length > UINT32_MAX)
if (length >= UINT32_MAX)
{
/* Codes_SRS_AMQPVALUE_01_401: [ If the string pointed to by value is longer than 2^32-1 then amqpvalue_create_symbol shall return NULL. ]*/
LogError("string too long to be represented as a symbol");
@ -5941,7 +5953,16 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
}
else
{
internal_decoder_data->decode_to_value->value.binary_value.bytes = (unsigned char*)malloc((size_t)internal_decoder_data->decode_to_value->value.binary_value.length + 1);
size_t malloc_size = (size_t)internal_decoder_data->decode_to_value->value.binary_value.length + 1;
if (malloc_size == 0)
{
internal_decoder_data->decode_to_value->value.binary_value.bytes = NULL;
LogError("Invalid binary_value size exceeded max allocation");
}
else
{
internal_decoder_data->decode_to_value->value.binary_value.bytes = (unsigned char*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.binary_value.bytes == NULL)
{
/* Codes_SRS_AMQPVALUE_01_326: [If any allocation failure occurs during decoding, amqpvalue_decode_bytes shall fail and return a non-zero value.] */
@ -5994,7 +6015,18 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
buffer++;
size--;
internal_decoder_data->decode_to_value->value.string_value.chars = (char*)malloc((size_t)internal_decoder_data->decode_value_state.string_value_state.length + 1);
size_t malloc_size = (size_t)internal_decoder_data->decode_value_state.string_value_state.length + 1;
// If the result of malloc_size is zero it means it had a type overflow (size_t is an unsigned type).
// It is very unlikely but could happen.
if (malloc_size == 0)
{
internal_decoder_data->decode_to_value->value.string_value.chars = NULL;
LogError("Invalid string size exceeded max allocation");
}
else
{
internal_decoder_data->decode_to_value->value.string_value.chars = (char*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.string_value.chars == NULL)
{
/* Codes_SRS_AMQPVALUE_01_326: [If any allocation failure occurs during decoding, amqpvalue_decode_bytes shall fail and return a non-zero value.] */
@ -6057,8 +6089,21 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
if (internal_decoder_data->bytes_decoded == 4)
{
internal_decoder_data->decode_to_value->value.string_value.chars = (char*)malloc((size_t)internal_decoder_data->decode_value_state.string_value_state.length + 1);
if (internal_decoder_data->decode_to_value->value.string_value.chars == NULL)
size_t malloc_size = (size_t)internal_decoder_data->decode_value_state.string_value_state.length + 1;
// If the result of malloc_size is zero it means it had a type overflow
// (size_t is an unsigned type). It is very unlikely but could happen.
if (malloc_size == 0)
{
internal_decoder_data->decode_to_value->value.string_value.chars = NULL;
LogError("Invalid string value size exceeded max allocation");
}
else
{
internal_decoder_data->decode_to_value->value.string_value.chars = (char*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.string_value.chars
== NULL)
{
/* Codes_SRS_AMQPVALUE_01_326: [If any allocation failure occurs during decoding, amqpvalue_decode_bytes shall fail and return a non-zero value.] */
internal_decoder_data->decoder_state = DECODER_STATE_ERROR;
@ -6123,7 +6168,19 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
buffer++;
size--;
internal_decoder_data->decode_to_value->value.symbol_value.chars = (char*)malloc((size_t)internal_decoder_data->decode_value_state.symbol_value_state.length + 1);
size_t malloc_size = (size_t)internal_decoder_data->decode_value_state.symbol_value_state.length + 1;
// If the result of malloc_size is zero it means it had a type overflow
// (size_t is an unsigned type). It is very unlikely but could happen.
if (malloc_size == 0)
{
internal_decoder_data->decode_to_value->value.symbol_value.chars = NULL;
LogError("Invalid symbol_value size exceeded max allocation");
}
else
{
internal_decoder_data->decode_to_value->value.symbol_value.chars = (char*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.symbol_value.chars == NULL)
{
/* Codes_SRS_AMQPVALUE_01_326: [If any allocation failure occurs during decoding, amqpvalue_decode_bytes shall fail and return a non-zero value.] */
@ -6186,8 +6243,19 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
if (internal_decoder_data->bytes_decoded == 4)
{
internal_decoder_data->decode_to_value->value.symbol_value.chars = (char*)malloc((size_t)internal_decoder_data->decode_value_state.symbol_value_state.length + 1);
if (internal_decoder_data->decode_to_value->value.symbol_value.chars == NULL)
size_t malloc_size = (size_t)internal_decoder_data->decode_value_state.symbol_value_state.length + 1;
// If the result of malloc_size is zero it means it had a type overflow
// (size_t is an unsigned type). It is very unlikely but could happen.
if (malloc_size == 0)
{
internal_decoder_data->decode_to_value->value.symbol_value.chars = NULL;
LogError("Invalid symbol value size exceeded max allocation");
}
else
{
internal_decoder_data->decode_to_value->value.symbol_value.chars = (char*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.symbol_value.chars == NULL)
{
/* Codes_SRS_AMQPVALUE_01_326: [If any allocation failure occurs during decoding, amqpvalue_decode_bytes shall fail and return a non-zero value.] */
internal_decoder_data->decoder_state = DECODER_STATE_ERROR;
@ -6531,7 +6599,18 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
internal_decoder_data->decode_to_value->value.map_value.pair_count /= 2;
internal_decoder_data->decode_to_value->value.map_value.pairs = (AMQP_MAP_KEY_VALUE_PAIR*)malloc(sizeof(AMQP_MAP_KEY_VALUE_PAIR) * ((size_t)internal_decoder_data->decode_to_value->value.map_value.pair_count * 2));
size_t malloc_size = safe_multiply_size_t(sizeof(AMQP_MAP_KEY_VALUE_PAIR), (size_t)internal_decoder_data->decode_to_value->value.map_value.pair_count);
malloc_size = safe_multiply_size_t(malloc_size, 2);
if (malloc_size == SIZE_MAX)
{
LogError("Invalid map_value size exceeded max allocation");
internal_decoder_data->decode_to_value->value.map_value.pairs = NULL;
}
else
{
internal_decoder_data->decode_to_value->value.map_value.pairs = (AMQP_MAP_KEY_VALUE_PAIR*)malloc(malloc_size);
}
if (internal_decoder_data->decode_to_value->value.map_value.pairs == NULL)
{
LogError("Could not allocate memory for map value items");
@ -6569,13 +6648,20 @@ static int internal_decoder_decode_bytes(INTERNAL_DECODER_DATA* internal_decoder
uint32_t i;
internal_decoder_data->decode_to_value->value.map_value.pair_count /= 2;
size_t malloc_size = safe_multiply_size_t((size_t)internal_decoder_data->decode_to_value->value.map_value.pair_count, 2);
malloc_size = safe_multiply_size_t(sizeof(AMQP_MAP_KEY_VALUE_PAIR), malloc_size);
if (internal_decoder_data->decode_to_value->value.map_value.pair_count > MAX_AMQPVALUE_ITEM_COUNT)
{
LogError("AMQP list map count exceeded MAX_AMQPVALUE_ITEM_COUNT");
result = MU_FAILURE;
}
else if (malloc_size == SIZE_MAX)
{
LogError("Invalid map_value size exceeded max allocation");
result = MU_FAILURE;
}
else if ((internal_decoder_data->decode_to_value->value.map_value.pairs =
(AMQP_MAP_KEY_VALUE_PAIR*)malloc(sizeof(AMQP_MAP_KEY_VALUE_PAIR) * ((size_t)internal_decoder_data->decode_to_value->value.map_value.pair_count * 2)))
(AMQP_MAP_KEY_VALUE_PAIR*)malloc(malloc_size))
== NULL)
{
LogError("Could not allocate memory for map value items");