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.

Author: douzzer

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 */

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;

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) {