Fix off-by-1 bug on provider_activate with custom error strings

Starting `cnt` from 1 would work if we weren't using cnt itself to
access elements of the array returned calling the provider callback.

As it is before this commit, we have 2 problems:
- first, in the unlikely case that the incoming array was "empty" (only
  contains the terminator item) we would skip past it and potentially
  end up with oob reads;
- otherwise, at the end of the while loop, `cnt` will be equal to the
  number of items in the input array, not 1 more. We then add 1 more to
  the zalloc call to account for the library name item, and we fill all
  of it (relying on zalloc to have zeroed the terminator item).
  The first read access that will read the list up to the terminator
  will result in a OOB read as we did not allocate enough space to also
  contain the terminator.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11427)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 2a463550d67b9131dd04cc17c12830d459e34283..ef8b3bf5b4e0ceb9338f65f53c137e77c43ebbdb 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -540,12 +540,13 @@ static int provider_activate(OSSL_PROVIDER *prov)
          * with the error library number, so we need to make a copy of that
          * array either way.
          */
-        cnt = 1;                 /* One for the terminating item */
+        cnt = 0;
         while (reasonstrings[cnt].id != 0) {
             if (ERR_GET_LIB(reasonstrings[cnt].id) != 0)
                 return 0;
             cnt++;
         }
+        cnt++;                   /* One for the terminating item */
 
         /* Allocate one extra item for the "library" name */
         prov->error_strings =