From fc7d4ffad4258a19585b4e27cb41df34c6b37e79 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 17 Dec 2025 11:07:22 -0600 Subject: [PATCH] PR#9545 20251211-DRBG-SHA2-smallstackcache-prealloc addressing peer review: clear dest if necessary in InitHandshakeHashesAndCopy(), style tweaks in random.c, explanatory comments in sha512.c. --- src/internal.c | 21 ++++++++++++++++----- wolfcrypt/src/random.c | 21 +++++++-------------- wolfcrypt/src/sha512.c | 19 +++++++++++++++++++ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/internal.c b/src/internal.c index 272d3ec37..da7557070 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7345,14 +7345,19 @@ int InitHandshakeHashesAndCopy(WOLFSSL* ssl, HS_Hashes* source, { int ret; - if (source == NULL) + if ((ssl == NULL) || (source == NULL) || (destination == NULL)) return BAD_FUNC_ARG; - /* Note we can't call InitHandshakeHashes() here, because the copy methods - * overwrite the entire dest low level hash struct. With some hashes and - * settings (e.g. SHA-2 hashes with WOLFSSL_SMALL_STACK_CACHE), internal - * scratch buffers are preallocated at init and will leak if overwritten. + /* If *destination is already allocated, its constituent hashes need to be + * freed, else they would leak. To keep things simple, we reuse + * FreeHandshakeHashes(), which deallocates *destination. */ + if (*destination != NULL) { + HS_Hashes* tmp = ssl->hsHashes; + ssl->hsHashes = *destination; + FreeHandshakeHashes(ssl); + ssl->hsHashes = tmp; + } /* allocate handshake hashes */ *destination = (HS_Hashes*)XMALLOC(sizeof(HS_Hashes), ssl->heap, @@ -7361,6 +7366,12 @@ int InitHandshakeHashesAndCopy(WOLFSSL* ssl, HS_Hashes* source, WOLFSSL_MSG("HS_Hashes Memory error"); return MEMORY_E; } + + /* Note we can't call InitHandshakeHashes() here, because the copy methods + * overwrite the entire dest low level hash struct. With some hashes and + * settings (e.g. SHA-2 hashes with WOLFSSL_SMALL_STACK_CACHE), internal + * scratch buffers are preallocated at init and will leak if overwritten. + */ XMEMSET(*destination, 0, sizeof(HS_Hashes)); /* now copy the source contents to the destination */ diff --git a/wolfcrypt/src/random.c b/wolfcrypt/src/random.c index 34e7c5487..2945a88b9 100644 --- a/wolfcrypt/src/random.c +++ b/wolfcrypt/src/random.c @@ -735,9 +735,7 @@ static int Hash_DRBG_Instantiate(DRBG_internal* drbg, const byte* seed, word32 s const byte* nonce, word32 nonceSz, void* heap, int devId) { -#ifdef WOLFSSL_SMALL_STACK_CACHE int ret = DRBG_FAILURE; -#endif XMEMSET(drbg, 0, sizeof(DRBG_internal)); drbg->heap = heap; @@ -757,10 +755,9 @@ static int Hash_DRBG_Instantiate(DRBG_internal* drbg, const byte* seed, word32 s return ret; #endif - if (seed == NULL) - return 0; - else - return Hash_DRBG_Init(drbg, seed, seedSz, nonce, nonceSz); + if (seed != NULL) + ret = Hash_DRBG_Init(drbg, seed, seedSz, nonce, nonceSz); + return ret; } /* Returns: DRBG_SUCCESS or DRBG_FAILURE */ @@ -815,11 +812,7 @@ static int _InitRng(WC_RNG* rng, byte* nonce, word32 nonceSz, int ret = 0; #ifdef HAVE_HASHDRBG word32 seedSz = SEED_SZ + SEED_BLOCK_SZ; - #ifdef WOLFSSL_SMALL_STACK - byte* seed = NULL; - #else - byte seed[MAX_SEED_SZ]; - #endif + WC_DECLARE_VAR(seed, byte, MAX_SEED_SZ, rng->heap); int drbg_instantiated = 0; #ifdef WOLFSSL_SMALL_STACK_CACHE int drbg_scratch_instantiated = 0; @@ -981,8 +974,7 @@ static int _InitRng(WC_RNG* rng, byte* nonce, word32 nonceSz, #ifdef WOLFSSL_SMALL_STACK if (ret == 0) { - seed = (byte*)XMALLOC(MAX_SEED_SZ, rng->heap, - DYNAMIC_TYPE_SEED); + WC_ALLOC_VAR_EX(seed, byte, MAX_SEED_SZ, rng->heap, DYNAMIC_TYPE_SEED, WC_DO_NOTHING); if (seed == NULL) { ret = MEMORY_E; rng->status = DRBG_FAILED; @@ -1418,7 +1410,8 @@ static int wc_RNG_HealthTest_ex_internal(DRBG_internal* drbg, } #ifdef WOLFSSL_SMALL_STACK_CACHE - (void)heap; (void)devId; + (void)heap; + (void)devId; if (Hash_DRBG_Init(drbg, seedA, seedASz, nonce, nonceSz) != 0) { goto exit_rng_ht; diff --git a/wolfcrypt/src/sha512.c b/wolfcrypt/src/sha512.c index cf67478b2..2b777beb7 100644 --- a/wolfcrypt/src/sha512.c +++ b/wolfcrypt/src/sha512.c @@ -870,6 +870,10 @@ static int InitSha512_Family(wc_Sha512* sha512, void* heap, int devId, sha512->heap = heap; #ifdef WOLFSSL_SMALL_STACK_CACHE + /* This allocation combines the customary W buffer used by + * _Transform_Sha512() with additional buffer space used by + * wc_Sha512Transform(). + */ sha512->W = (word64 *)XMALLOC((sizeof(word64) * 16) + WC_SHA512_BLOCK_SIZE, sha512->heap, DYNAMIC_TYPE_DIGEST); if (sha512->W == NULL) @@ -1702,6 +1706,9 @@ int wc_Sha512Transform(wc_Sha512* sha, const unsigned char* data) #if defined(WOLFSSL_SMALL_STACK_CACHE) if (sha->W == NULL) return BAD_FUNC_ARG; + /* Skip over the initial `W' buffer at the start (used by + * _Transform_Sha512()). + */ buffer = sha->W + 16; #elif defined(WOLFSSL_SMALL_STACK) buffer = (word64*)XMALLOC(WC_SHA512_BLOCK_SIZE, sha->heap, @@ -1873,6 +1880,10 @@ static int InitSha384(wc_Sha384* sha384) #ifdef WOLFSSL_SMALL_STACK_CACHE if (sha384->W == NULL) { + /* This allocation combines the customary W buffer used by + * _Transform_Sha512() with additional buffer space used by + * wc_Sha512Transform(). + */ sha384->W = (word64 *)XMALLOC((sizeof(word64) * 16) + WC_SHA512_BLOCK_SIZE, sha384->heap, DYNAMIC_TYPE_DIGEST); if (sha384->W == NULL) @@ -2232,6 +2243,10 @@ int wc_Sha512Copy(wc_Sha512* src, wc_Sha512* dst) XMEMCPY(dst, src, sizeof(wc_Sha512)); #ifdef WOLFSSL_SMALL_STACK_CACHE + /* This allocation combines the customary W buffer used by + * _Transform_Sha512() with additional buffer space used by + * wc_Sha512Transform(). + */ dst->W = (word64 *)XMALLOC((sizeof(word64) * 16) + WC_SHA512_BLOCK_SIZE, dst->heap, DYNAMIC_TYPE_DIGEST); if (dst->W == NULL) { @@ -2667,6 +2682,10 @@ int wc_Sha384Copy(wc_Sha384* src, wc_Sha384* dst) XMEMCPY(dst, src, sizeof(wc_Sha384)); #ifdef WOLFSSL_SMALL_STACK_CACHE + /* This allocation combines the customary W buffer used by + * _Transform_Sha512() with additional buffer space used by + * wc_Sha512Transform(). + */ dst->W = (word64 *)XMALLOC((sizeof(word64) * 16) + WC_SHA384_BLOCK_SIZE, dst->heap, DYNAMIC_TYPE_DIGEST); if (dst->W == NULL) {