commit 8c7e925b5dab1f826f08f38e4b9e1543b8413476
author David Benjamin <davidben@google.com> Tue May 23 11:45:09 2023 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue May 23 16:58:21 2023 +0000
tree ce4eda6c0acd4f7a14e5695cf832c6936010c376
parent cf3851c6c9380368373ac127cde1f4aa7159fba3

Bound STACK_OF(T) sizes by int

Although we've switched STACK_OF(T) to use size_t, OpenSSL used int
pervasively. Much of crypto/x509 and third-party callers use int
indices. As much of that is in the public API now, ensure that
STACK_OF(T) can never exceed INT_MAX elements.

Bug: 516
Change-Id: I26b8fe590655f8c3e449b749b5d0222e28c413f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60065
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>

crypto/stack/stack.c

diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index 7f60b2e..c81afcb 100644
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -57,7 +57,9 @@
 #include <openssl/stack.h>
 
 #include <assert.h>
+#include <limits.h>
 
+#include <openssl/err.h>
 #include <openssl/mem.h>
 
 #include "../internal.h"
@@ -161,6 +163,11 @@
     return 0;
   }
 
+  if (sk->num >= INT_MAX) {
+    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_OVERFLOW);
+    return 0;
+  }
+
   if (sk->num_alloc <= sk->num + 1) {
     // Attempt to double the size of the array.
     size_t new_alloc = sk->num_alloc << 1;

include/openssl/stack.h

diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index 59b1c5e..2774e86 100644
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -138,7 +138,8 @@
 // NULL on allocation failure.
 STACK_OF(SAMPLE) *sk_SAMPLE_new_null(void);
 
-// sk_SAMPLE_num returns the number of elements in |sk|.
+// sk_SAMPLE_num returns the number of elements in |sk|. It is safe to cast this
+// value to |int|. |sk| is guaranteed to have at most |INT_MAX| elements.
 size_t sk_SAMPLE_num(const STACK_OF(SAMPLE) *sk);
 
 // sk_SAMPLE_zero resets |sk| to the empty state but does nothing to free the