[Cryptech-Commits] [sw/libhal] 01/03: Add buffer overflow checks before allocating stack arrays.

[git at cryptech.is]

This is an automated email from the git hooks/post-receive script.

paul at psgd.org pushed a commit to branch master
in repository sw/libhal.

commit 2b4972ee5c17b64162333fdd1d023158e35c8c1a
Author: Paul Selkirk <paul at psgd.org>
AuthorDate: Tue Oct 23 18:01:02 2018 -0400

    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)));