wolfSSL/wolfssl
1
0
Fork 1
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-08-04 04:45:44 +02:00
Code Issues Packages Projects Releases Wiki Activity

rsa: address review - drop XMEMSET, free uninitialized temporaries instead

Browse Source 
Restore the original short-circuit XMALLOC chains and leave the zeroing to
mp_init_multi(). On a partial allocation failure mp_init_multi() is skipped,
so nothing is initialized: free whatever was allocated right there and NULL
the pointers, so the shared cleanup at the end of the function never calls
mp_clear()/mp_forcezero() on an allocated-but-uninitialized mp_int.

No extra mp_init() calls, no XMEMSET, and the returned error codes are
unchanged.
This commit is contained in:
Daniele Lacamera
2026-07-28 18:05:02 +02:00
parent 2e985f46c3
commit 94014ee5ca
1 changed files with 35 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
wolfcrypt/src/rsa.c
+35 -37
View File
@@ -5062,19 +5062,18 @@ static int wc_CompareDiffPQ(mp_int* p, mp_int* q, int size, int* valid)
return BAD_FUNC_ARG;
#ifdef WOLFSSL_SMALL_STACK
c = (mp_int *)XMALLOC(sizeof(*c), NULL, DYNAMIC_TYPE_WOLF_BIGINT);
d = (mp_int *)XMALLOC(sizeof(*d), NULL, DYNAMIC_TYPE_WOLF_BIGINT);
/* Zero any struct that was allocated so the cleanup path's mp_clear()/
* mp_forcezero() are safe even when the sibling allocation fails and the
* mp_init_multi() below is skipped on MEMORY_E. Without this, clearing an
* allocated-but-uninitialized mp_int reads a garbage ->used and corrupts
* the heap. */
if (c != NULL)
XMEMSET(c, 0, sizeof(*c));
if (d != NULL)
XMEMSET(d, 0, sizeof(*d));
if (c == NULL || d == NULL)
if (((c = (mp_int *)XMALLOC(sizeof(*c), NULL, DYNAMIC_TYPE_WOLF_BIGINT)) == NULL) ||
((d = (mp_int *)XMALLOC(sizeof(*d), NULL, DYNAMIC_TYPE_WOLF_BIGINT)) == NULL)) {
/* mp_init_multi() below is skipped, so nothing was initialized: free
* what was allocated here and NULL the pointers. The cleanup at the
* end must not see an allocated-but-uninitialized mp_int - clearing
* one reads a garbage used/size and corrupts the heap. */
XFREE(c, NULL, DYNAMIC_TYPE_WOLF_BIGINT);
XFREE(d, NULL, DYNAMIC_TYPE_WOLF_BIGINT);
c = NULL;
d = NULL;
ret = MEMORY_E;
}
else
ret = 0;
@@ -5215,17 +5214,16 @@ static int _CheckProbablePrime(mp_int* p, mp_int* q, mp_int* e, int nlen,
*isPrime = MP_NO;
#ifdef WOLFSSL_SMALL_STACK
tmp1 = (mp_int *)XMALLOC(sizeof(*tmp1), NULL, DYNAMIC_TYPE_WOLF_BIGINT);
tmp2 = (mp_int *)XMALLOC(sizeof(*tmp2), NULL, DYNAMIC_TYPE_WOLF_BIGINT);
/* Zero any allocated struct so the notOkay cleanup's mp_forcezero()/
* mp_clear() are safe when the sibling allocation fails and the
* mp_init_multi() below is skipped: clearing an allocated-but-
* uninitialized mp_int reads a garbage ->used and corrupts the heap. */
if (tmp1 != NULL)
XMEMSET(tmp1, 0, sizeof(*tmp1));
if (tmp2 != NULL)
XMEMSET(tmp2, 0, sizeof(*tmp2));
if (tmp1 == NULL || tmp2 == NULL) {
if (((tmp1 = (mp_int *)XMALLOC(sizeof(*tmp1), NULL, DYNAMIC_TYPE_WOLF_BIGINT)) == NULL) ||
((tmp2 = (mp_int *)XMALLOC(sizeof(*tmp2), NULL, DYNAMIC_TYPE_WOLF_BIGINT)) == NULL)) {
/* mp_init_multi() below is skipped, so nothing was initialized: free
* what was allocated here and NULL the pointers. The notOkay cleanup
* must not see an allocated-but-uninitialized mp_int - clearing one
* reads a garbage used/size and corrupts the heap. */
XFREE(tmp1, NULL, DYNAMIC_TYPE_WOLF_BIGINT);
XFREE(tmp2, NULL, DYNAMIC_TYPE_WOLF_BIGINT);
tmp1 = NULL;
tmp2 = NULL;
ret = MEMORY_E;
goto notOkay;
}
@@ -5328,21 +5326,21 @@ int wc_CheckProbablePrime_ex(const byte* pRaw, word32 pRawSz,
#ifdef WOLFSSL_SMALL_STACK
p = (mp_int *)XMALLOC(sizeof(*p), NULL, DYNAMIC_TYPE_RSA_BUFFER);
q = (mp_int *)XMALLOC(sizeof(*q), NULL, DYNAMIC_TYPE_RSA_BUFFER);
e = (mp_int *)XMALLOC(sizeof(*e), NULL, DYNAMIC_TYPE_RSA_BUFFER);
/* Zero any allocated struct so the cleanup path's mp_forcezero()/
* mp_clear() are safe when a later allocation in the chain fails and the
* mp_init_multi() below is skipped on MEMORY_E: clearing an allocated-but-
* uninitialized mp_int reads a garbage ->used and corrupts the heap. */
if (p != NULL)
XMEMSET(p, 0, sizeof(*p));
if (q != NULL)
XMEMSET(q, 0, sizeof(*q));
if (e != NULL)
XMEMSET(e, 0, sizeof(*e));
if (p == NULL || q == NULL || e == NULL)
if (((p = (mp_int *)XMALLOC(sizeof(*p), NULL, DYNAMIC_TYPE_RSA_BUFFER)) == NULL) ||
((q = (mp_int *)XMALLOC(sizeof(*q), NULL, DYNAMIC_TYPE_RSA_BUFFER)) == NULL) ||
((e = (mp_int *)XMALLOC(sizeof(*e), NULL, DYNAMIC_TYPE_RSA_BUFFER)) == NULL)) {
/* mp_init_multi() below is skipped, so nothing was initialized: free
* what was allocated here and NULL the pointers. The cleanup at the
* end must not see an allocated-but-uninitialized mp_int - clearing
* one reads a garbage used/size and corrupts the heap. */
XFREE(p, NULL, DYNAMIC_TYPE_RSA_BUFFER);
XFREE(q, NULL, DYNAMIC_TYPE_RSA_BUFFER);
XFREE(e, NULL, DYNAMIC_TYPE_RSA_BUFFER);
p = NULL;
q = NULL;
e = NULL;
ret = MEMORY_E;
}
else
ret = 0;
if (ret == 0)