commit 7ff871acbe1c58d142f089e228626e6a63ad5fe0
author David Benjamin <davidben@google.com> Wed Nov 23 12:02:37 2022 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Dec 07 19:01:54 2022 +0000
tree 1bcbf6b13c7121c1653fa8db7d95de5e6c9b5d8d
parent 726585c58ec89f65ab669187665232790cbf3d02

Fix -Wshorten-64-to-32 errors in ex_data.c.

Bug: 516
Change-Id: Ifd381d1a2ed30aed6ffe84eb83d8fb4d93ec02ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55451
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>

crypto/ex_data.c

diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 71d60a5..532a3ad 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -109,6 +109,8 @@
 #include <openssl/ex_data.h>
 
 #include <assert.h>
+#include <limits.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <openssl/crypto.h>
@@ -149,24 +151,41 @@
     ex_data_class->meth = sk_CRYPTO_EX_DATA_FUNCS_new_null();
   }
 
-  if (ex_data_class->meth == NULL ||
-      !sk_CRYPTO_EX_DATA_FUNCS_push(ex_data_class->meth, funcs)) {
+  if (ex_data_class->meth == NULL) {
     OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
-    OPENSSL_free(funcs);
     goto err;
   }
 
-  *out_index = sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth) - 1 +
+  // The index must fit in |int|.
+  if (sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth) >
+          (size_t)(INT_MAX - ex_data_class->num_reserved)) {
+    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_OVERFLOW);
+    goto err;
+  }
+
+  if (!sk_CRYPTO_EX_DATA_FUNCS_push(ex_data_class->meth, funcs)) {
+    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
+    goto err;
+  }
+  funcs = NULL;  // |sk_CRYPTO_EX_DATA_FUNCS_push| takes ownership.
+
+  *out_index = (int)sk_CRYPTO_EX_DATA_FUNCS_num(ex_data_class->meth) - 1 +
                ex_data_class->num_reserved;
   ret = 1;
 
 err:
   CRYPTO_STATIC_MUTEX_unlock_write(&ex_data_class->lock);
+  OPENSSL_free(funcs);
   return ret;
 }
 
 int CRYPTO_set_ex_data(CRYPTO_EX_DATA *ad, int index, void *val) {
-  int n, i;
+  if (index < 0) {
+    // A caller that can accidentally pass in an invalid index into this
+    // function will hit an memory error if |index| happened to be valid, and
+    // expected |val| to be of a different type.
+    abort();
+  }
 
   if (ad->sk == NULL) {
     ad->sk = sk_void_new_null();
@@ -176,17 +195,15 @@
     }
   }
 
-  n = sk_void_num(ad->sk);
-
   // Add NULL values until the stack is long enough.
-  for (i = n; i <= index; i++) {
+  for (size_t i = sk_void_num(ad->sk); i <= (size_t)index; i++) {
     if (!sk_void_push(ad->sk, NULL)) {
       OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
       return 0;
     }
   }
 
-  sk_void_set(ad->sk, index, val);
+  sk_void_set(ad->sk, (size_t)index, val);
   return 1;
 }
 
@@ -242,7 +259,10 @@
     return;
   }
 
-  for (size_t i = 0; i < sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
+  // |CRYPTO_get_ex_new_index| will not allocate indices beyond |INT_MAX|.
+  assert(sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers) <=
+         (size_t)(INT_MAX - ex_data_class->num_reserved));
+  for (int i = 0; i < (int)sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
     CRYPTO_EX_DATA_FUNCS *func_pointer =
         sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i);
     if (func_pointer->free_func) {

include/openssl/ex_data.h

diff --git a/include/openssl/ex_data.h b/include/openssl/ex_data.h
index 102f8a8..8f2f98b 100644
--- a/include/openssl/ex_data.h
+++ b/include/openssl/ex_data.h
@@ -145,7 +145,7 @@
                                          CRYPTO_EX_free *free_func);
 
 // TYPE_set_ex_data sets an extra data pointer on |t|. The |index| argument
-// should have been returned from a previous call to |TYPE_get_ex_new_index|.
+// must have been returned from a previous call to |TYPE_get_ex_new_index|.
 OPENSSL_EXPORT int TYPE_set_ex_data(TYPE *t, int index, void *arg);
 
 // TYPE_get_ex_data returns an extra data pointer for |t|, or NULL if no such