Address clusterfuzz issue regarding a null read in provider fuzzer

[nhorman]

We have a new clusterfuzz issue in the provider fuzzer: https://oss-fuzz.com/testcase-detail/5068142433861632

It appears related to #680

Unsure of the root cause, but it looks like we jumped to the 0 page, implying that a function pointer is null. Odd part is that the stack doesn't have a function pointer at the location last indicated.

Needs investigation

[Sashan]

I've finally managed to dig out a stack trace from reproducer:

#0  0x00005555559def0b in EVP_DigestUpdate () at crypto/evp/digest.c:428
#1  0x0000555555b35ba8 in kmac_derive () at providers/implementations/kdfs/kbkdf.c:268
#2  kbkdf_derive () at providers/implementations/kdfs/kbkdf.c:305
#3  0x00005555559471f0 in do_evp_kdf () at fuzz/provider.c:449
#4  0x000055555594470d in FuzzerTestOneInput () at fuzz/provider.c:619
#5  0x0000555555966c81 in ExecuteCallback () at /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614
#6  0x0000555555951415 in RunOneTest () at /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327
#7  0x0000555555956eab in FuzzerDriver () at /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862
#8  0x00005555559832a3 in main () at /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20

We die at very last line of EVP_DigestUpdate() here at line 128:

414
415         if (ctx->digest == NULL
416                 || ctx->digest->prov == NULL
417                 || (ctx->flags & EVP_MD_CTX_FLAG_NO_INIT) != 0)
418             goto legacy;
419
420         if (ctx->digest->dupdate == NULL) {
421             ERR_raise(ERR_LIB_EVP, EVP_R_UPDATE_ERROR);
422             return 0;
423         }
424         return ctx->digest->dupdate(ctx->algctx, data, count);
425
426         /* Code below to be removed when legacy support is dropped. */
427      legacy:
428         return ctx->update(ctx, data, count);

the ctx parameter as we enter the function looks as follows:

(gdb) p *((EVP_MD_CTX *)$rdi)
$2 = {reqdigest = 0x0, digest = 0x0, engine = 0x0, flags = 0, md_data = 0x0, pctx = 0x0, update = 0x0, algctx = 0x0, 
  fetched_digest = 0x0}

So the code starts to move sideways at line 415 because member digest is NULL. The function assumes we deal with legacy provider and jumps to legacy target, where we die immediately on NULL pointer dereference. I`m not expert here so can not how fuzzer managed to get us that far. Revealing parameters generated by fuzzer is hard because I need to do manual stack unwinding. jumping to frames in debugger does not seem to work for fuzzer binaries, the gdb is no able to extract local variables in frames. not sure why perhaps binary got linked with static crypto is to blame here. anyway this simple tweak will make fuzzer happy:

--- a/crypto/evp/digest.c
+++ b/crypto/evp/digest.c
@@ -425,7 +425,7 @@ int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *data, size_t count)

     /* Code below to be removed when legacy support is dropped. */
  legacy:
-    return ctx->update(ctx, data, count);
+    return ctx->update != NULL ? ctx->update(ctx, data, count) : 0;
 }

 /* The caller can assume that this removes any secret data from the context */

Someone with more expertise should confirm fix is valid.

IMO this issue has no implications on security, the context is corrupted.

[t8m]

@Sashan Could you please find out which KDF algorithm was selected?

Actually it is obvious from the stack trace that it was KBKDF

[t8m]

So although IMO adding the NULL check to the EVP_DigestUpdate makes sense, we should also fix the KBKDF provider implementation to avoid calling EVP_MAC_update() or EVP_MAC_final() before previous call to EVP_MAC_init().

And we should probably also fix KMAC provider to report error if mac_update or mac_final is called before init.