Merge pull request #4712 from rizlik/aes_free_fix

Fix potential resources leaks

src/keys.c

@@ -2476,17 +2476,25 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs,
         int aesRet = 0;
 
         if (enc) {
-            if (enc->aes == NULL)
+            if (enc->aes == NULL) {
                 enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (enc->aes == NULL)
                     return MEMORY_E;
+            } else {
+                wc_AesFree(enc->aes);
+            }
+
             XMEMSET(enc->aes, 0, sizeof(Aes));
         }
         if (dec) {
-            if (dec->aes == NULL)
+            if (dec->aes == NULL) {
                 dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (dec->aes == NULL)
                     return MEMORY_E;
+            } else {
+                wc_AesFree(dec->aes);
+            }
+
             XMEMSET(dec->aes, 0, sizeof(Aes));
         }
         if (enc) {
@@ -2553,17 +2561,25 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs,
         int gcmRet;
 
         if (enc) {
-            if (enc->aes == NULL)
+            if (enc->aes == NULL) {
                 enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (enc->aes == NULL)
                     return MEMORY_E;
+            } else {
+                wc_AesFree(enc->aes);
+            }
+
             XMEMSET(enc->aes, 0, sizeof(Aes));
         }
         if (dec) {
-            if (dec->aes == NULL)
+            if (dec->aes == NULL) {
                 dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (dec->aes == NULL)
                     return MEMORY_E;
+            } else {
+                wc_AesFree(dec->aes);
+            }
+
             XMEMSET(dec->aes, 0, sizeof(Aes));
         }
 
@@ -2653,17 +2669,24 @@ static int SetKeys(Ciphers* enc, Ciphers* dec, Keys* keys, CipherSpecs* specs,
         int CcmRet;
 
         if (enc) {
-            if (enc->aes == NULL)
+            if (enc->aes == NULL) {
                 enc->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (enc->aes == NULL)
                     return MEMORY_E;
+            } else {
+                wc_AesFree(enc->aes);
+            }
+
             XMEMSET(enc->aes, 0, sizeof(Aes));
         }
         if (dec) {
-            if (dec->aes == NULL)
+            if (dec->aes == NULL) {
                 dec->aes = (Aes*)XMALLOC(sizeof(Aes), heap, DYNAMIC_TYPE_CIPHER);
                 if (dec->aes == NULL)
                 return MEMORY_E;
+            } else {
+                wc_AesFree(dec->aes);
+            }
             XMEMSET(dec->aes, 0, sizeof(Aes));
         }
 

tests/api.c

@@ -14998,16 +14998,21 @@ static int test_wc_InitCmac (void)
     ret = wc_InitCmac(&cmac1, key1, key1Sz, type, NULL);
 #endif
 #ifdef WOLFSSL_AES_192
-    if (ret == 0)
+    if (ret == 0) {
+        wc_AesFree(&cmac1.aes);
         ret = wc_InitCmac(&cmac2, key2, key2Sz, type, NULL);
+    }
 #endif
 #ifdef WOLFSSL_AES_256
-    if (ret == 0)
+    if (ret == 0) {
+        wc_AesFree(&cmac2.aes);
         ret = wc_InitCmac(&cmac3, key3, key3Sz, type, NULL);
+    }
 #endif
 
     /* Test bad args. */
     if (ret == 0) {
+        wc_AesFree(&cmac3.aes);
         ret = wc_InitCmac(NULL, key3, key3Sz, type, NULL);
         if (ret == BAD_FUNC_ARG) {
             ret = wc_InitCmac(&cmac3, NULL, key3Sz, type, NULL);
@@ -15084,6 +15089,7 @@ static int test_wc_CmacUpdate (void)
         } else if (ret == 0) {
             ret = WOLFSSL_FATAL_ERROR;
         }
+        wc_AesFree(&cmac.aes);
     }
 
     printf(resultFmt, ret == 0 ? passed : failed);
@@ -15205,6 +15211,8 @@ static int test_wc_AesCmacGenerate (void)
     ret = wc_CmacUpdate(&cmac, msg, msgSz);
     if (ret != 0) {
         return ret;
+    } else {
+        wc_AesFree(&cmac.aes);
     }
 
     printf(testingFmt, "wc_AesCmacGenerate()");
@@ -17489,7 +17497,9 @@ static int test_wc_GmacUpdate (void)
         if (ret == 0) {
             ret = XMEMCMP(tag1, tagOut, sizeof(tag1));
         }
+        wc_AesFree(&gmac.aes);
     }
+
 #endif
 
 #ifdef WOLFSSL_AES_192
@@ -17503,6 +17513,7 @@ static int test_wc_GmacUpdate (void)
     }
     if (ret == 0) {
         ret = XMEMCMP(tagOut2, tag2, sizeof(tag2));
+        wc_AesFree(&gmac.aes);
     }
 #endif
 

wolfcrypt/src/aes.c

@@ -9415,6 +9415,10 @@ int wc_AesGcmDecryptFinal(Aes* aes, const byte* authTag, word32 authTagSz)
         }
     }
 
+    /* reset the state */
+    if (ret == 0)
+        wc_AesFree(aes);
+
     return ret;
 }
 #endif /* HAVE_AES_DECRYPT || HAVE_AESGCM_DECRYPT */
@@ -10384,6 +10388,7 @@ void wc_AesFree(Aes* aes)
     !defined(WOLFSSL_AESNI)
     if (aes->streamData != NULL) {
         XFREE(aes->streamData, aes->heap, DYNAMIC_TYPE_AES);
+        aes->streamData = NULL;
     }
 #endif
 

wolfcrypt/src/cmac.c

@@ -249,6 +249,7 @@ int wc_CmacFinal(Cmac* cmac, byte* out, word32* outSz)
     }
 #endif
 
+    wc_AesFree(&cmac->aes);
     ForceZero(cmac, sizeof(Cmac));
 
     return ret;

wolfcrypt/src/wc_encrypt.c

@@ -625,6 +625,8 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt,
         case PBE_AES256_CBC:
         case PBE_AES128_CBC:
         {
+            int free_aes;
+
 #ifdef WOLFSSL_SMALL_STACK
             Aes *aes;
             aes = (Aes *)XMALLOC(sizeof *aes, NULL, DYNAMIC_TYPE_AES);
@@ -633,8 +635,10 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt,
 #else
             Aes aes[1];
 #endif
+            free_aes = 0;
             ret = wc_AesInit(aes, NULL, INVALID_DEVID);
             if (ret == 0) {
+                free_aes = 1;
                 if (enc) {
                     ret = wc_AesSetKey(aes, key, derivedLen, cbcIv,
                                                                 AES_ENCRYPTION);
@@ -650,6 +654,8 @@ int wc_CryptKey(const char* password, int passwordSz, byte* salt,
                 else
                     ret = wc_AesCbcDecrypt(aes, input, input, length);
             }
+            if (free_aes)
+                wc_AesFree(aes);
             ForceZero(aes, sizeof(Aes));
 #ifdef WOLFSSL_SMALL_STACK
             XFREE(aes, NULL, DYNAMIC_TYPE_AES);

wolfcrypt/test/test.c

@@ -7007,6 +7007,8 @@ EVP_TEST_END:
 
   out:
 
+        wc_AesFree(enc);
+        wc_AesFree(dec);
 #ifdef WOLFSSL_SMALL_STACK
     if (enc)
         XFREE(enc, HEAP_HINT, DYNAMIC_TYPE_AES);
@@ -8043,6 +8045,8 @@ static int aes_xts_128_test(void)
         ERROR_OUT(-5402, out);
 
     XMEMSET(buf, 0, sizeof(buf));
+    wc_AesXtsFree(aes);
+
     if (wc_AesXtsSetKey(aes, k1, sizeof(k1), AES_ENCRYPTION,
             HEAP_HINT, devId) != 0)
         ERROR_OUT(-5403, out);
@@ -8089,6 +8093,7 @@ static int aes_xts_128_test(void)
         ERROR_OUT(-5410, out);
     if (XMEMCMP(p1, buf, AES_BLOCK_SIZE))
         ERROR_OUT(-5411, out);
+    wc_AesXtsFree(aes);
 
     /* fail case with decrypting using wrong key */
     XMEMSET(buf, 0, sizeof(buf));
@@ -8243,6 +8248,7 @@ static int aes_xts_256_test(void)
         ERROR_OUT(-5501, out);
     if (XMEMCMP(c2, buf, sizeof(c2)))
         ERROR_OUT(-5502, out);
+    wc_AesXtsFree(aes);
 
     XMEMSET(buf, 0, sizeof(buf));
     if (wc_AesXtsSetKey(aes, k1, sizeof(k1), AES_ENCRYPTION,
@@ -8291,6 +8297,7 @@ static int aes_xts_256_test(void)
         ERROR_OUT(-5510, out);
     if (XMEMCMP(p1, buf, AES_BLOCK_SIZE))
         ERROR_OUT(-5511, out);
+    wc_AesXtsFree(aes);
 
     XMEMSET(buf, 0, sizeof(buf));
     if (wc_AesXtsSetKey(aes, k2, sizeof(k2), AES_DECRYPTION,
@@ -8398,6 +8405,7 @@ static int aes_xts_sector_test(void)
         ERROR_OUT(-5601, out);
     if (XMEMCMP(c1, buf, AES_BLOCK_SIZE))
         ERROR_OUT(-5602, out);
+    wc_AesXtsFree(aes);
 
     /* decrypt test */
     XMEMSET(buf, 0, sizeof(buf));
@@ -8427,6 +8435,7 @@ static int aes_xts_sector_test(void)
         ERROR_OUT(-5607, out);
     if (XMEMCMP(c2, buf, sizeof(c2)))
         ERROR_OUT(-5608, out);
+    wc_AesXtsFree(aes);
 
     /* decrypt test */
     XMEMSET(buf, 0, sizeof(buf));
@@ -10399,7 +10408,7 @@ WOLFSSL_TEST_SUBROUTINE int gmac_test(void)
 #endif
 
     XMEMSET(gmac, 0, sizeof *gmac); /* clear context */
-    (void)wc_AesInit((Aes*)gmac, HEAP_HINT, INVALID_DEVID); /* Make sure devId updated */
+    (void)wc_AesInit(&gmac->aes, HEAP_HINT, INVALID_DEVID); /* Make sure devId updated */
     XMEMSET(tag, 0, sizeof(tag));
     wc_GmacSetKey(gmac, k1, sizeof(k1));
     wc_GmacUpdate(gmac, iv1, sizeof(iv1), a1, sizeof(a1), tag, sizeof(t1));
@@ -10460,6 +10469,7 @@ WOLFSSL_TEST_SUBROUTINE int gmac_test(void)
     ret = 0;
 
   out:
+    wc_AesFree(&gmac->aes);
 #ifdef WOLFSSL_SMALL_STACK
     XFREE(gmac, HEAP_HINT, DYNAMIC_TYPE_AES);
 #endif
@@ -10613,6 +10623,7 @@ WOLFSSL_TEST_SUBROUTINE int aesccm_test(void)
     XMEMSET(c2, 0, sizeof(c2));
     if (XMEMCMP(p2, c2, sizeof(p2)))
         ERROR_OUT(-6507, out);
+    wc_AesFree(enc);
 
     XMEMSET(enc, 0, sizeof(Aes)); /* clear context */
     XMEMSET(t2, 0, sizeof(t2));
@@ -10726,6 +10737,8 @@ WOLFSSL_TEST_SUBROUTINE int aesccm_test(void)
     if (result != 0)
         ERROR_OUT(-6526, out);
 
+    wc_AesFree(enc);
+
     ret = 0;
 
   out:

wolfssl/test.h

@@ -3579,6 +3579,11 @@ static WC_INLINE void FreeAtomicUser(WOLFSSL* ssl)
 
     /* Encrypt-Then-MAC callbacks use same contexts. */
 
+    if (encCtx->keySetup  == 1)
+        wc_AesFree(&encCtx->aes);
+    if (decCtx->keySetup  == 1)
+        wc_AesFree(&decCtx->aes);
+
     free(decCtx);
     free(encCtx);
 }