Skip to content

Fix potential memory leak when copying into existing SHA contexts and zero init tmpSha #9829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
night1rider wants to merge 7 commits into
base: master
Choose a base branch
from
+389 −5
Draft

Fix potential memory leak when copying into existing SHA contexts and zero init tmpSha #9829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c8c323
Fix potential memory leak when copying into existing SHA contexts and…
night1rider Feb 24, 2026
58a6158
Fix potential memory leak in SHA Copy and zero-initialize temp GetHas…
night1rider Feb 24, 2026
455b3c6
memset 0 the temp contexts
night1rider Feb 24, 2026
47037dd
Add Free(dst) + XMEMSET before XMEMCPY in all wc_ hash Copy functions…
night1rider Feb 25, 2026
d99fe3b
Fix SHA3/Shake copy cleanup tests to heap-allocate shaCopy to avoid e…
night1rider Feb 25, 2026
1177068
Revert "Fix SHA3/Shake copy cleanup tests to heap-allocate shaCopy to…
night1rider Feb 25, 2026
7f06ade
Use WC_DECLARE_VAR/WC_ALLOC_VAR for shaCopy in SHA3/Shake tests to av…
night1rider Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12372,6 +12372,7 @@ static int BuildMD5(WOLFSSL* ssl, Hashes* hashes, const byte* sender)
#else
wc_Md5 md5[1];
#endif
XMEMSET(md5, 0, sizeof(wc_Md5));

/* make md5 inner */
ret = wc_Md5Copy(&ssl->hsHashes->hashMd5, md5);
Expand Down Expand Up @@ -12417,6 +12418,7 @@ static int BuildSHA(WOLFSSL* ssl, Hashes* hashes, const byte* sender)
#else
wc_Sha sha[1];
#endif
XMEMSET(sha, 0, sizeof(wc_Sha));
/* make sha inner */
ret = wc_ShaCopy(&ssl->hsHashes->hashSha, sha); /* Save current position */
if (ret == 0)
Expand Down Expand Up @@ -23926,6 +23928,7 @@ static int BuildMD5_CertVerify(const WOLFSSL* ssl, byte* digest)
#else
wc_Md5 md5[1];
#endif
XMEMSET(md5, 0, sizeof(wc_Md5));

/* make md5 inner */
ret = wc_Md5Copy(&ssl->hsHashes->hashMd5, md5); /* Save current position */
Expand Down Expand Up @@ -23969,6 +23972,7 @@ static int BuildSHA_CertVerify(const WOLFSSL* ssl, byte* digest)
#else
wc_Sha sha[1];
#endif
XMEMSET(sha, 0, sizeof(wc_Sha));

/* make sha inner */
ret = wc_ShaCopy(&ssl->hsHashes->hashSha, sha); /* Save current position */
Expand Down
2 changes: 2 additions & 0 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11982,6 +11982,8 @@ static int ExpectedResumptionSecret(WOLFSSL* ssl)
Digest digest;
static byte header[] = { 0x14, 0x00, 0x00, 0x00 };

XMEMSET(&digest, 0, sizeof(Digest));

/* Copy the running hash so we can restore it after. */
switch (ssl->specs.mac_algorithm) {
#ifndef NO_SHA256
Expand Down
3 changes: 3 additions & 0 deletions wolfcrypt/src/evp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5846,6 +5846,9 @@ void wolfSSL_EVP_init(void)
if (out->pctx == NULL)
return WOLFSSL_FAILURE;
}
/* Zero hash context after shallow copy to prevent shared sub-pointers
* with src. The hash Copy function will perform the proper deep copy. */
XMEMSET(&out->hash, 0, sizeof(out->hash));
return wolfSSL_EVP_MD_Copy_Hasher(out, (WOLFSSL_EVP_MD_CTX*)in);
}
#ifndef NO_AES
Expand Down
5 changes: 5 additions & 0 deletions wolfcrypt/src/hmac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ int wc_HmacCopy(Hmac* src, Hmac* dst) {

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

/* Zero hash context after shallow copy to prevent shared sub-pointers
* (e.g., msg, W buffers) with src. The hash Copy function will perform
* the proper deep copy. */
XMEMSET(&dst->hash, 0, sizeof(wc_HmacHash));

ret = HmacKeyCopyHash(src->macType, &src->hash, &dst->hash);

if (ret != 0)
Expand Down
5 changes: 5 additions & 0 deletions wolfcrypt/src/md5.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,11 @@ int wc_Md5Copy(wc_Md5* src, wc_Md5* dst)
if (src == NULL || dst == NULL)
return BAD_FUNC_ARG;

/* Free dst resources before copy to prevent memory leaks (e.g.,
* hardware contexts). XMEMCPY overwrites dst. */
wc_Md5Free(dst);
XMEMSET(dst, 0, sizeof(wc_Md5));

XMEMCPY(dst, src, sizeof(wc_Md5));

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_MD5)
Expand Down
7 changes: 6 additions & 1 deletion wolfcrypt/src/sha.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ int wc_ShaGetHash(wc_Sha* sha, byte* hash)
return BAD_FUNC_ARG;
}

WC_ALLOC_VAR_EX(tmpSha, wc_Sha, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
WC_CALLOC_VAR_EX(tmpSha, wc_Sha, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
return MEMORY_E);

ret = wc_ShaCopy(sha, tmpSha);
Expand Down Expand Up @@ -1172,6 +1172,11 @@ int wc_ShaCopy(wc_Sha* src, wc_Sha* dst)
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */

/* Free dst resources before copy to prevent memory leaks (e.g., msg
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
wc_ShaFree(dst);
XMEMSET(dst, 0, sizeof(wc_Sha));

XMEMCPY(dst, src, sizeof(wc_Sha));

#if defined(WOLFSSL_SILABS_SE_ACCEL) && defined(WOLFSSL_SILABS_SE_ACCEL_3)
Expand Down
14 changes: 12 additions & 2 deletions wolfcrypt/src/sha256.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2546,7 +2546,7 @@ int wc_Sha224_Grow(wc_Sha224* sha224, const byte* in, int inSz)
return BAD_FUNC_ARG;
}

WC_ALLOC_VAR_EX(tmpSha224, wc_Sha224, 1, NULL,
WC_CALLOC_VAR_EX(tmpSha224, wc_Sha224, 1, NULL,
DYNAMIC_TYPE_TMP_BUFFER, return MEMORY_E);

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

/* Free dst resources before copy to prevent memory leaks (e.g., msg
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
wc_Sha224Free(dst);
XMEMSET(dst, 0, sizeof(wc_Sha224));

XMEMCPY(dst, src, sizeof(wc_Sha224));

#ifdef WOLFSSL_SMALL_STACK_CACHE
Expand Down Expand Up @@ -2691,7 +2696,7 @@ int wc_Sha256GetHash(wc_Sha256* sha256, byte* hash)
return BAD_FUNC_ARG;
}

WC_ALLOC_VAR_EX(tmpSha256, wc_Sha256, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
WC_CALLOC_VAR_EX(tmpSha256, wc_Sha256, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
return MEMORY_E);

ret = wc_Sha256Copy(sha256, tmpSha256);
Expand Down Expand Up @@ -2728,6 +2733,11 @@ int wc_Sha256Copy(wc_Sha256* src, wc_Sha256* dst)
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */

/* Free dst resources before copy to prevent memory leaks (e.g., msg
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
wc_Sha256Free(dst);
XMEMSET(dst, 0, sizeof(wc_Sha256));

XMEMCPY(dst, src, sizeof(wc_Sha256));

#ifdef WOLFSSL_MAXQ10XX_CRYPTO
Expand Down
5 changes: 5 additions & 0 deletions wolfcrypt/src/sha3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,11 @@ static int wc_Sha3Copy(wc_Sha3* src, wc_Sha3* dst)
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */

/* Free dst resources before copy to prevent memory leaks (e.g.,
* hardware contexts). XMEMCPY overwrites dst. */
wc_Sha3Free(dst);
XMEMSET(dst, 0, sizeof(wc_Sha3));
Copy link
Contributor

@SparkiDev SparkiDev Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why memset when replacing every byte in memcpy?

Copy link
Contributor Author

@night1rider night1rider Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it is redundant, and I initially did not have the xmemset in earlier commits (the SHA and SHA-256 copy code paths). I decided to add it to follow the convention that if we free a context, we should probably re-init it in some capacity to a known state before operating with it. I originally thought about adding our InitSha after the free, but decided that seemed like too much. I followed the other code patterns where we use xmemset to get the context initialized. I wasn't sure if copying the state of another context via memcpy is considered as 'known good' compared to a zeroized struct. I am happy to remove the xmemset since the memcpy is there. The xmemset would likely be optimized away by a compiler anyways.


XMEMCPY(dst, src, sizeof(wc_Sha3));

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_SHA3)
Expand Down
14 changes: 12 additions & 2 deletions wolfcrypt/src/sha512.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2206,7 +2206,7 @@ static int Sha512_Family_GetHash(wc_Sha512* sha512, byte* hash,
return BAD_FUNC_ARG;
}

WC_ALLOC_VAR_EX(tmpSha512, wc_Sha512, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
WC_CALLOC_VAR_EX(tmpSha512, wc_Sha512, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
return MEMORY_E);

/* copy this sha512 into tmpSha */
Expand Down Expand Up @@ -2249,6 +2249,11 @@ int wc_Sha512Copy(wc_Sha512* src, wc_Sha512* dst)
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */

/* Free dst resources before copy to prevent memory leaks (e.g., msg
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
wc_Sha512Free(dst);
XMEMSET(dst, 0, sizeof(wc_Sha512));

XMEMCPY(dst, src, sizeof(wc_Sha512));
#ifdef WOLFSSL_SMALL_STACK_CACHE
/* This allocation combines the customary W buffer used by
Expand Down Expand Up @@ -2649,7 +2654,7 @@ int wc_Sha384GetHash(wc_Sha384* sha384, byte* hash)
return BAD_FUNC_ARG;
}

WC_ALLOC_VAR_EX(tmpSha384, wc_Sha384, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
WC_CALLOC_VAR_EX(tmpSha384, wc_Sha384, 1, NULL, DYNAMIC_TYPE_TMP_BUFFER,
return MEMORY_E);

/* copy this sha384 into tmpSha */
Expand Down Expand Up @@ -2687,6 +2692,11 @@ int wc_Sha384Copy(wc_Sha384* src, wc_Sha384* dst)
ret = 0; /* Reset ret to 0 to avoid returning the callback error code */
#endif /* WOLF_CRYPTO_CB && WOLF_CRYPTO_CB_COPY */

/* Free dst resources before copy to prevent memory leaks (e.g., msg
* buffer, W cache, hardware contexts). XMEMCPY overwrites dst. */
wc_Sha384Free(dst);
XMEMSET(dst, 0, sizeof(wc_Sha384));

XMEMCPY(dst, src, sizeof(wc_Sha384));

#ifdef WOLFSSL_SMALL_STACK_CACHE
Expand Down
Loading
Loading