From 2b4972ee5c17b64162333fdd1d023158e35c8c1a Mon Sep 17 00:00:00 2001 From: Paul Selkirk Date: Tue, 23 Oct 2018 18:01:02 -0400 Subject: Add buffer overflow checks before allocating stack arrays. This fixes CT-01-005: OOB writes through dynamic stack allocations (Critical) --- rpc_misc.c | 2 +- rpc_server.c | 31 ++++++++++++++++++++++--------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/rpc_misc.c b/rpc_misc.c index c27913c..50ee3ac 100644 --- a/rpc_misc.c +++ b/rpc_misc.c @@ -44,7 +44,7 @@ static hal_error_t get_version(uint32_t *version) static hal_error_t get_random(void *buffer, const size_t length) { - if (buffer == NULL || length == 0) + if (buffer == NULL) return HAL_ERROR_IMPOSSIBLE; return hal_get_random(NULL, buffer, length); diff --git a/rpc_server.c b/rpc_server.c index 5a06e37..2f07ec0 100644 --- a/rpc_server.c +++ b/rpc_server.c @@ -68,7 +68,7 @@ static hal_error_t get_random(const uint8_t **iptr, const uint8_t * const ilimit check(hal_xdr_decode_int(iptr, ilimit, &length)); /* sanity check length */ - if (length == 0 || length > (uint32_t)(olimit - *optr - nargs(1))) + if (nargs(1) + pad(length) > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; /* get the data directly into the output buffer */ @@ -168,7 +168,7 @@ static hal_error_t hash_get_digest_algorithm_id(const uint8_t **iptr, const uint check(hal_xdr_decode_int(iptr, ilimit, &alg)); check(hal_xdr_decode_int(iptr, ilimit, &len_max)); /* sanity check len_max */ - if (len_max > (uint32_t)(olimit - *optr - nargs(1))) + if (nargs(1) + pad(len_max) > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; /* get the data directly into the output buffer */ @@ -245,7 +245,7 @@ static hal_error_t hash_finalize(const uint8_t **iptr, const uint8_t * const ili check(hal_xdr_decode_int(iptr, ilimit, &hash.handle)); check(hal_xdr_decode_int(iptr, ilimit, &length)); /* sanity check length */ - if (length > (uint32_t)(olimit - *optr - nargs(1))) + if (nargs(1) + pad(length) > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; /* get the data directly into the output buffer */ @@ -493,7 +493,7 @@ static hal_error_t pkey_get_public_key(const uint8_t **iptr, const uint8_t * con check(hal_xdr_decode_int(iptr, ilimit, &pkey.handle)); check(hal_xdr_decode_int(iptr, ilimit, &len_max)); /* sanity check len_max */ - if (len_max > (uint32_t)(olimit - *optr - nargs(1))) + if (nargs(1) + pad(len_max) > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; /* get the data directly into the output buffer */ @@ -523,7 +523,7 @@ static hal_error_t pkey_sign(const uint8_t **iptr, const uint8_t * const ilimit, check(hal_xdr_decode_variable_opaque_ptr(iptr, ilimit, &input, &input_len)); check(hal_xdr_decode_int(iptr, ilimit, &sig_max)); /* sanity check sig_max */ - if (sig_max > (uint32_t)(olimit - *optr - nargs(1))) + if (nargs(1) + pad(sig_max) > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; /* get the data directly into the output buffer */ @@ -576,6 +576,9 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit check(hal_xdr_decode_int(iptr, ilimit, &flags)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); + if (nargs(2 * attributes_len) > (uint32_t)(ilimit - *iptr)) + return HAL_ERROR_RPC_PACKET_OVERFLOW; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (size_t i = 0; i < attributes_len; i++) { @@ -597,6 +600,9 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit const hal_uuid_t * const previous_uuid = (const void *) previous_uuid_ptr; + if (nargs(2) + result_max * (nargs(1) + sizeof(hal_uuid_t)) > (uint32_t)(olimit - *optr)) + return HAL_ERROR_RPC_PACKET_OVERFLOW; + hal_uuid_t result[result_max]; unsigned result_len, ustate = state; @@ -628,6 +634,9 @@ static hal_error_t pkey_set_attributes(const uint8_t **iptr, const uint8_t * con check(hal_xdr_decode_int(iptr, ilimit, &pkey.handle)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); + if (nargs(2 * attributes_len) > (uint32_t)(ilimit - *iptr)) + return HAL_ERROR_RPC_PACKET_OVERFLOW; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (size_t i = 0; i < attributes_len; i++) { @@ -656,7 +665,7 @@ static hal_error_t pkey_get_attributes(const uint8_t **iptr, const uint8_t * con { hal_client_handle_t client; hal_pkey_handle_t pkey; - uint32_t attributes_len, u32; + uint32_t attributes_len, attributes_buffer_len; uint8_t *optr_orig = *optr; hal_error_t err; @@ -664,14 +673,15 @@ static hal_error_t pkey_get_attributes(const uint8_t **iptr, const uint8_t * con check(hal_xdr_decode_int(iptr, ilimit, &pkey.handle)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); + if (nargs(1 + attributes_len) > (uint32_t)(ilimit - *iptr)) + return HAL_ERROR_RPC_PACKET_OVERFLOW; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (size_t i = 0; i < attributes_len; i++) check(hal_xdr_decode_int(iptr, ilimit, &attributes[i].type)); - check(hal_xdr_decode_int(iptr, ilimit, &u32)); - - const size_t attributes_buffer_len = u32; + check(hal_xdr_decode_int(iptr, ilimit, &attributes_buffer_len)); if (nargs(1 + 2 * attributes_len) + attributes_buffer_len > (uint32_t)(olimit - *optr)) return HAL_ERROR_RPC_PACKET_OVERFLOW; @@ -715,6 +725,9 @@ static hal_error_t pkey_export(const uint8_t **iptr, const uint8_t * const ilimi check(hal_xdr_decode_int(iptr, ilimit, &pkcs8_max)); check(hal_xdr_decode_int(iptr, ilimit, &kek_max)); + if (nargs(2) + pad(pkcs8_max) + pad(kek_max) > (uint32_t)(olimit - *optr)) + return HAL_ERROR_RPC_PACKET_OVERFLOW; + uint8_t pkcs8[pkcs8_max], kek[kek_max]; check(hal_rpc_pkey_export(pkey, kekek, pkcs8, &pkcs8_len, sizeof(pkcs8), kek, &kek_len, sizeof(kek))); -- cgit v1.2.3