Skip to content

Commit 215fe13

Browse files
SparkiDevSparkiDev
authored
Merge pull request #9829 from night1rider/tmpSha-fixes
Fix potential memory leak when copying into existing SHA contexts and zero init tmpSha
2 parents cba9ffd + 69ddefb commit 215fe13

12 files changed

Lines changed: 397 additions & 5 deletions

File tree

‎src/internal.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12368,6 +12368,7 @@ static int BuildMD5(WOLFSSL* ssl, Hashes* hashes, const byte* sender)
1236812368
#else
1236912369
wc_Md5 md5[1];
1237012370
#endif
12371+
XMEMSET(md5, 0, sizeof(wc_Md5));
1237112372

1237212373
/* make md5 inner */
1237312374
ret = wc_Md5Copy(&ssl->hsHashes->hashMd5, md5);
@@ -12413,6 +12414,7 @@ static int BuildSHA(WOLFSSL* ssl, Hashes* hashes, const byte* sender)
1241312414
#else
1241412415
wc_Sha sha[1];
1241512416
#endif
12417+
XMEMSET(sha, 0, sizeof(wc_Sha));
1241612418
/* make sha inner */
1241712419
ret = wc_ShaCopy(&ssl->hsHashes->hashSha, sha); /* Save current position */
1241812420
if (ret == 0)
@@ -23919,6 +23921,7 @@ static int BuildMD5_CertVerify(const WOLFSSL* ssl, byte* digest)
2391923921
#else
2392023922
wc_Md5 md5[1];
2392123923
#endif
23924+
XMEMSET(md5, 0, sizeof(wc_Md5));
2392223925

2392323926
/* make md5 inner */
2392423927
ret = wc_Md5Copy(&ssl->hsHashes->hashMd5, md5); /* Save current position */
@@ -23962,6 +23965,7 @@ static int BuildSHA_CertVerify(const WOLFSSL* ssl, byte* digest)
2396223965
#else
2396323966
wc_Sha sha[1];
2396423967
#endif
23968+
XMEMSET(sha, 0, sizeof(wc_Sha));
2396523969

2396623970
/* make sha inner */
2396723971
ret = wc_ShaCopy(&ssl->hsHashes->hashSha, sha); /* Save current position */

‎src/tls13.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11983,6 +11983,8 @@ static int ExpectedResumptionSecret(WOLFSSL* ssl)
1198311983
Digest digest;
1198411984
static byte header[] = { 0x14, 0x00, 0x00, 0x00 };
1198511985

11986+
XMEMSET(&digest, 0, sizeof(Digest));
11987+
1198611988
/* Copy the running hash so we can restore it after. */
1198711989
switch (ssl->specs.mac_algorithm) {
1198811990
#ifndef NO_SHA256

‎wolfcrypt/src/evp.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5846,6 +5846,9 @@ void wolfSSL_EVP_init(void)
58465846
if (out->pctx == NULL)
58475847
return WOLFSSL_FAILURE;
58485848
}
5849+
/* Zero hash context after shallow copy to prevent shared sub-pointers
5850+
* with src. The hash Copy function will perform the proper deep copy. */
5851+
XMEMSET(&out->hash, 0, sizeof(out->hash));
58495852
return wolfSSL_EVP_MD_Copy_Hasher(out, (WOLFSSL_EVP_MD_CTX*)in);
58505853
}
58515854
#ifndef NO_AES

‎wolfcrypt/src/hmac.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,11 @@ int wc_HmacCopy(Hmac* src, Hmac* dst) {
331331

332332
XMEMCPY(dst, src, sizeof(*dst));
333333

334+
/* Zero hash context after shallow copy to prevent shared sub-pointers
335+
* (e.g., msg, W buffers) with src. The hash Copy function will perform
336+
* the proper deep copy. */
337+
XMEMSET(&dst->hash, 0, sizeof(wc_HmacHash));
338+
334339
ret = HmacKeyCopyHash(src->macType, &src->hash, &dst->hash);
335340

336341
if (ret != 0)

‎wolfcrypt/src/md5.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ int wc_Md5GetHash(wc_Md5* md5, byte* hash)
522522
if (md5 == NULL || hash == NULL)
523523
return BAD_FUNC_ARG;
524524

525+
XMEMSET(&tmpMd5, 0, sizeof(tmpMd5));
525526
ret = wc_Md5Copy(md5, &tmpMd5);
526527
if (ret == 0) {
527528
ret = wc_Md5Final(&tmpMd5, hash);
@@ -537,6 +538,9 @@ int wc_Md5Copy(wc_Md5* src, wc_Md5* dst)
537538
if (src == NULL || dst == NULL)
538539
return BAD_FUNC_ARG;
539540

541+
/* Free dst resources before copy to prevent memory leaks (e.g.,
542+
* hardware contexts). XMEMCPY overwrites dst. */
543+
wc_Md5Free(dst);
540544
XMEMCPY(dst, src, sizeof(wc_Md5));
541545

542546
#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_MD5)

‎wolfcrypt/src/port/riscv/riscv-64-sha256.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,7 @@ int wc_Sha256GetHash(wc_Sha256* sha256, byte* hash)
10311031
}
10321032
else {
10331033
wc_Sha256 tmpSha256;
1034+
XMEMSET(&tmpSha256, 0, sizeof(tmpSha256));
10341035
/* Create a copy of the hash to finalize. */
10351036
ret = wc_Sha256Copy(sha256, &tmpSha256);
10361037
if (ret == 0) {
@@ -1350,6 +1351,7 @@ int wc_Sha224GetHash(wc_Sha224* sha224, byte* hash)
13501351
}
13511352
else {
13521353
wc_Sha224 tmpSha224;
1354+
XMEMSET(&tmpSha224, 0, sizeof(tmpSha224));
13531355
/* Create a copy of the hash to finalize. */
13541356
ret = wc_Sha224Copy(sha224, &tmpSha224);
13551357
if (ret == 0) {

‎wolfcrypt/src/port/riscv/riscv-64-sha512.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,7 @@ int wc_Sha512GetHash(wc_Sha512* sha512, byte* hash)
11401140
}
11411141
else {
11421142
wc_Sha512 tmpSha512;
1143+
XMEMSET(&tmpSha512, 0, sizeof(tmpSha512));
11431144
/* Create a copy of the hash to finalize. */
11441145
ret = wc_Sha512Copy(sha512, &tmpSha512);
11451146
if (ret == 0) {
@@ -1357,6 +1358,7 @@ int wc_Sha512_224GetHash(wc_Sha512* sha512, byte* hash)
13571358
}
13581359
else {
13591360
wc_Sha512 tmpSha512;
1361+
XMEMSET(&tmpSha512, 0, sizeof(tmpSha512));
13601362
/* Create a copy of the hash to finalize. */
13611363
ret = wc_Sha512Copy(sha512, &tmpSha512);
13621364
if (ret == 0) {
@@ -1456,6 +1458,7 @@ int wc_Sha512_256GetHash(wc_Sha512* sha512, byte* hash)
14561458
}
14571459
else {
14581460
wc_Sha512 tmpSha512;
1461+
XMEMSET(&tmpSha512, 0, sizeof(tmpSha512));
14591462
/* Create a copy of the hash to finalize. */
14601463
ret = wc_Sha512Copy(sha512, &tmpSha512);
14611464
if (ret == 0) {
@@ -1671,6 +1674,7 @@ int wc_Sha384GetHash(wc_Sha384* sha384, byte* hash)
16711674
}
16721675
else {
16731676
wc_Sha384 tmpSha384;
1677+
XMEMSET(&tmpSha384, 0, sizeof(tmpSha384));
16741678
/* Create a copy of the hash to finalize. */
16751679
ret = wc_Sha384Copy(sha384, &tmpSha384);
16761680
if (ret == 0) {

‎wolfcrypt/src/sha.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1137,7 +1137,7 @@ int wc_ShaGetHash(wc_Sha* sha, byte* hash)
11371137
return BAD_FUNC_ARG;
11381138
}
11391139

1140-
WC_ALLOC_VAR_EX(tmpSha, wc_Sha, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
1140+
WC_CALLOC_VAR_EX(tmpSha, wc_Sha, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
11411141
return MEMORY_E);
11421142

11431143
ret = wc_ShaCopy(sha, tmpSha);
@@ -1172,6 +1172,9 @@ int wc_ShaCopy(wc_Sha* src, wc_Sha* dst)
11721172
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
11731173
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */
11741174

1175+
/* Free dst resources before copy to prevent memory leaks (e.g., msg
1176+
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
1177+
wc_ShaFree(dst);
11751178
XMEMCPY(dst, src, sizeof(wc_Sha));
11761179

11771180
#if defined(WOLFSSL_SILABS_SE_ACCEL) && defined(WOLFSSL_SILABS_SE_ACCEL_3)

‎wolfcrypt/src/sha256.c‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2546,7 +2546,7 @@ int wc_Sha224_Grow(wc_Sha224* sha224, const byte* in, int inSz)
25462546
return BAD_FUNC_ARG;
25472547
}
25482548

2549-
WC_ALLOC_VAR_EX(tmpSha224, wc_Sha224, 1, NULL,
2549+
WC_CALLOC_VAR_EX(tmpSha224, wc_Sha224, 1, NULL,
25502550
DYNAMIC_TYPE_TMP_BUFFER, return MEMORY_E);
25512551

25522552
ret = wc_Sha224Copy(sha224, tmpSha224);
@@ -2582,6 +2582,9 @@ int wc_Sha224_Grow(wc_Sha224* sha224, const byte* in, int inSz)
25822582
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
25832583
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */
25842584

2585+
/* Free dst resources before copy to prevent memory leaks (e.g., msg
2586+
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
2587+
wc_Sha224Free(dst);
25852588
XMEMCPY(dst, src, sizeof(wc_Sha224));
25862589

25872590
#ifdef WOLFSSL_SMALL_STACK_CACHE
@@ -2691,7 +2694,7 @@ int wc_Sha256GetHash(wc_Sha256* sha256, byte* hash)
26912694
return BAD_FUNC_ARG;
26922695
}
26932696

2694-
WC_ALLOC_VAR_EX(tmpSha256, wc_Sha256, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
2697+
WC_CALLOC_VAR_EX(tmpSha256, wc_Sha256, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
26952698
return MEMORY_E);
26962699

26972700
ret = wc_Sha256Copy(sha256, tmpSha256);
@@ -2728,6 +2731,9 @@ int wc_Sha256Copy(wc_Sha256* src, wc_Sha256* dst)
27282731
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
27292732
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */
27302733

2734+
/* Free dst resources before copy to prevent memory leaks (e.g., msg
2735+
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
2736+
wc_Sha256Free(dst);
27312737
XMEMCPY(dst, src, sizeof(wc_Sha256));
27322738

27332739
#ifdef WOLFSSL_MAXQ10XX_CRYPTO

‎wolfcrypt/src/sha3.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,9 @@ static int wc_Sha3Copy(wc_Sha3* src, wc_Sha3* dst)
13061306
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
13071307
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */
13081308

1309+
/* Free dst resources before copy to prevent memory leaks (e.g.,
1310+
* hardware contexts). XMEMCPY overwrites dst. */
1311+
wc_Sha3Free(dst);
13091312
XMEMCPY(dst, src, sizeof(wc_Sha3));
13101313

13111314
#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_SHA3)
@@ -1342,6 +1345,7 @@ static int wc_Sha3GetHash(wc_Sha3* sha3, byte* hash, byte p, byte len)
13421345
if (sha3 == NULL || hash == NULL)
13431346
return BAD_FUNC_ARG;
13441347

1348+
XMEMSET(&tmpSha3, 0, sizeof(tmpSha3));
13451349
ret = wc_Sha3Copy(sha3, &tmpSha3);
13461350
if (ret == 0) {
13471351
ret = wc_Sha3Final(&tmpSha3, hash, p, len);

0 commit comments

Comments
 (0)