From fd03868b34faaa711403d9ac0843d33811fbf961 Mon Sep 17 00:00:00 2001 From: Nicola Tuveri Date: Fri, 27 Mar 2020 15:39:34 +0100 Subject: [PATCH] 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 Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/11427) --- crypto/provider_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 2a463550d6..ef8b3bf5b4 100644 --- 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 = -- 2.25.1