aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Selkirk <paul@psgd.org>2018-10-23 18:01:02 -0400
committerPaul Selkirk <paul@psgd.org>2018-10-25 17:16:10 -0400
commit2b4972ee5c17b64162333fdd1d023158e35c8c1a (patch)
tree8f177275758482779eb6225b9fdfb5766996967d
parent7219e78b554a7b7e792a344b0c6cf133affe88b2 (diff)
Add buffer overflow checks before allocating stack arrays.
This fixes CT-01-005: OOB writes through dynamic stack allocations (Critical)
-rw-r--r--rpc_misc.c2
-rw-r--r--rpc_server.c31
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)));