feedc0de/wolfssl
1
0
Fork 0
forked from wolfSSL/wolfssl
Code Pull Requests Activity

more fixes guided by clang-tidy heap analyzer using clang-20.0.0_pre20250104:

Browse Source 
wolfcrypt/src/integer.c: add additional guards against OOB access from uint wraps and null derefs of mp_int.dp, and refactor mp_grow() and mp_init_size() to use XMEMSET, for the benefit of clang-tidy.  in mp_grow(), fix the condition for the realloc to assure always evaluated if a->alloc == 0.

wolfcrypt/src/asn.c: fix wc_CreatePKCS8Key() so that *outSz is always assigned when LENGTH_ONLY_E is returned.

wolfcrypt/src/pkcs7.c: remove redundant inner condition in wc_PKCS7_EncodeAuthEnvelopedData(), added in previous commit and caught on review by Jacob (thanks!).

wolfcrypt/src/sp_int.c: in sp_mont_norm(), add another suppression for the same false positive in sp_mul() suppressed in previous commit.

wolfcrypt/src/srp.c: refactor SrpHashSize() to return ALGO_ID_E rather than 0 when unknown/uncompiled alg is requested.
This commit is contained in:
Daniel Pouzzner
2025-01-10 15:48:05 -06:00
parent 7cd2fd3617
commit b16bedf82a
5 changed files with 82 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
wolfcrypt/src/asn.c
View File

@@ -7429,7 +7429,7 @@ int wc_CreatePKCS8Key(byte* out, word32* outSz, byte* key, word32 keySz,
/* Get the size of the DER encoding. */ /* Get the size of the DER encoding. */
ret = SizeASN_Items(pkcs8KeyASN, dataASN, pkcs8KeyASN_Length-1, &sz); ret = SizeASN_Items(pkcs8KeyASN, dataASN, pkcs8KeyASN_Length-1, &sz);
} }
if (ret == 0) { if ((ret == 0) || (ret == WC_NO_ERR_TRACE(LENGTH_ONLY_E))) {
/* Always return the calculated size. */ /* Always return the calculated size. */
*outSz = (word32)sz; *outSz = (word32)sz;
} }

31
wolfcrypt/src/integer.c
View File

@@ -409,11 +409,10 @@ int mp_copy (const mp_int * a, mp_int * b)
/* grow as required */ /* grow as required */
int mp_grow (mp_int * a, int size) int mp_grow (mp_int * a, int size)
{ {
int i;
mp_digit *tmp; mp_digit *tmp;
/* if the alloc size is smaller alloc more ram */ /* if the alloc size is smaller alloc more ram */
if (a->alloc < size || size == 0) { if ((a->alloc < size) || (size == 0) || (a->alloc == 0)) {
/* ensure there are always at least MP_PREC digits extra on top */ /* ensure there are always at least MP_PREC digits extra on top */
size += (MP_PREC * 2) - (size % MP_PREC); size += (MP_PREC * 2) - (size % MP_PREC);
@@ -434,14 +433,11 @@ int mp_grow (mp_int * a, int size)
a->dp = tmp; a->dp = tmp;
/* zero excess digits */ /* zero excess digits */
i = a->alloc; XMEMSET(&a->dp[a->alloc], 0, sizeof (mp_digit) * (size - a->alloc));
a->alloc = size; a->alloc = size;
for (; i < a->alloc; i++) {
a->dp[i] = 0;
} }
} else if (a->dp == NULL) {
else if ((a->alloc > 0) && (a->dp == NULL)) { /* opportunistic sanity check for null a->dp with nonzero a->alloc */
/* opportunistic sanity check on a->dp */
return MP_VAL; return MP_VAL;
} }
return MP_OKAY; return MP_OKAY;
@@ -1762,9 +1758,9 @@ int s_mp_add (mp_int * a, mp_int * b, mp_int * c)
/* destination */ /* destination */
tmpc = c->dp; tmpc = c->dp;
/* sanity-check dp pointers from a and b. */ /* sanity-check dp pointers. */
if ((min_ab > 0) && if ((min_ab > 0) &&
((tmpa == NULL) || (tmpb == NULL))) ((tmpa == NULL) || (tmpb == NULL) || (tmpc == NULL)))
{ {
return MP_VAL; return MP_VAL;
} }
@@ -3308,6 +3304,10 @@ int mp_div_3 (mp_int * a, mp_int *c, mp_digit * d)
q.used = a->used; q.used = a->used;
q.sign = a->sign; q.sign = a->sign;
w = 0; w = 0;
if (a->used == 0)
return MP_VAL;
for (ix = a->used - 1; ix >= 0; ix--) { for (ix = a->used - 1; ix >= 0; ix--) {
w = (w << ((mp_word)DIGIT_BIT)) | ((mp_word)a->dp[ix]); w = (w << ((mp_word)DIGIT_BIT)) | ((mp_word)a->dp[ix]);
@@ -3350,8 +3350,6 @@ int mp_div_3 (mp_int * a, mp_int *c, mp_digit * d)
/* init an mp_init for a given size */ /* init an mp_init for a given size */
int mp_init_size (mp_int * a, int size) int mp_init_size (mp_int * a, int size)
{ {
int x;
/* pad size so there are always extra digits */ /* pad size so there are always extra digits */
size += (MP_PREC * 2) - (size % MP_PREC); size += (MP_PREC * 2) - (size % MP_PREC);
@@ -3371,9 +3369,7 @@ int mp_init_size (mp_int * a, int size)
#endif #endif
/* zero the digits */ /* zero the digits */
for (x = 0; x < size; x++) { XMEMSET(a->dp, 0, sizeof (mp_digit) * size);
a->dp[x] = 0;
}
return MP_OKAY; return MP_OKAY;
} }
@@ -4699,8 +4695,11 @@ static int mp_div_d (mp_int * a, mp_digit b, mp_int * c, mp_digit * d)
} }
} }
w = 0; w = 0;
if (a->used == 0)
return MP_VAL;
for (ix = a->used - 1; ix >= 0; ix--) { for (ix = a->used - 1; ix >= 0; ix--) {
w = (w << ((mp_word)DIGIT_BIT)) | ((mp_word)a->dp[ix]); w = (w << ((mp_word)DIGIT_BIT)) | ((mp_word)a->dp[ix]);

2
wolfcrypt/src/pkcs7.c
View File

@@ -13115,11 +13115,9 @@ int wc_PKCS7_EncodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* output,
if (unauthAttribsSz > 0) { if (unauthAttribsSz > 0) {
XMEMCPY(output + idx, unauthAttribSet, unauthAttribsSetSz); XMEMCPY(output + idx, unauthAttribSet, unauthAttribsSetSz);
idx += (int)unauthAttribsSetSz; idx += (int)unauthAttribsSetSz;
if (unauthAttribsSz > 0) {
XMEMCPY(output + idx, flatUnauthAttribs, unauthAttribsSz); XMEMCPY(output + idx, flatUnauthAttribs, unauthAttribsSz);
idx += (int)unauthAttribsSz; idx += (int)unauthAttribsSz;
} }
}
XFREE(flatUnauthAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(flatUnauthAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7);

5
wolfcrypt/src/sp_int.c
View File

@@ -17982,9 +17982,14 @@ int sp_mont_norm(sp_int* norm, const sp_int* m)
if (err == MP_OKAY) { if (err == MP_OKAY) {
/* Find top bit and ensure norm has enough space. */ /* Find top bit and ensure norm has enough space. */
bits = (unsigned int)sp_count_bits(m); bits = (unsigned int)sp_count_bits(m);
/* NOLINTBEGIN(clang-analyzer-core.UndefinedBinaryOperatorResult) */
/* clang-tidy falsely believes that norm->size was corrupted by the
* _sp_copy() to "Set real working value to base." in _sp_exptmod_ex().
*/
if (bits >= (unsigned int)norm->size * SP_WORD_SIZE) { if (bits >= (unsigned int)norm->size * SP_WORD_SIZE) {
err = MP_VAL; err = MP_VAL;
} }
/* NOLINTEND(clang-analyzer-core.UndefinedBinaryOperatorResult) */
} }
if (err == MP_OKAY) { if (err == MP_OKAY) {
/* Round up for case when m is less than a word - no advantage in using /* Round up for case when m is less than a word - no advantage in using

83
wolfcrypt/src/srp.c
View File

@@ -152,39 +152,39 @@ static int SrpHashFinal(SrpHash* hash, byte* digest)
} }
} }
static word32 SrpHashSize(SrpType type) static int SrpHashSize(SrpType type)
{ {
switch (type) { switch (type) {
case SRP_TYPE_SHA: case SRP_TYPE_SHA:
#ifndef NO_SHA #ifndef NO_SHA
return WC_SHA_DIGEST_SIZE; return WC_SHA_DIGEST_SIZE;
#else #else
return 0; return ALGO_ID_E;
#endif #endif
case SRP_TYPE_SHA256: case SRP_TYPE_SHA256:
#ifndef NO_SHA256 #ifndef NO_SHA256
return WC_SHA256_DIGEST_SIZE; return WC_SHA256_DIGEST_SIZE;
#else #else
return 0; return ALGO_ID_E;
#endif #endif
case SRP_TYPE_SHA384: case SRP_TYPE_SHA384:
#ifdef WOLFSSL_SHA384 #ifdef WOLFSSL_SHA384
return WC_SHA384_DIGEST_SIZE; return WC_SHA384_DIGEST_SIZE;
#else #else
return 0; return ALGO_ID_E;
#endif #endif
case SRP_TYPE_SHA512: case SRP_TYPE_SHA512:
#ifdef WOLFSSL_SHA512 #ifdef WOLFSSL_SHA512
return WC_SHA512_DIGEST_SIZE; return WC_SHA512_DIGEST_SIZE;
#else #else
return 0; return ALGO_ID_E;
#endif #endif
default: default:
return 0; return ALGO_ID_E;
} }
} }
@@ -353,7 +353,8 @@ int wc_SrpSetParams(Srp* srp, const byte* N, word32 nSz,
byte digest2[SRP_MAX_DIGEST_SIZE]; byte digest2[SRP_MAX_DIGEST_SIZE];
byte pad = 0; byte pad = 0;
int r; int r;
word32 i, j = 0; word32 i;
int hashSize = 0;
if (!srp || !N || !g || !salt || nSz < gSz) if (!srp || !N || !g || !salt || nSz < gSz)
return BAD_FUNC_ARG; return BAD_FUNC_ARG;
@@ -361,6 +362,10 @@ int wc_SrpSetParams(Srp* srp, const byte* N, word32 nSz,
if (!srp->user) if (!srp->user)
return SRP_CALL_ORDER_E; return SRP_CALL_ORDER_E;
hashSize = SrpHashSize(srp->type);
if (hashSize < 0)
return hashSize;
/* Set N */ /* Set N */
if (mp_read_unsigned_bin(&srp->N, N, nSz) != MP_OKAY) if (mp_read_unsigned_bin(&srp->N, N, nSz) != MP_OKAY)
return MP_READ_E; return MP_READ_E;
@@ -414,7 +419,7 @@ int wc_SrpSetParams(Srp* srp, const byte* N, word32 nSz,
/* digest1 = H(N) ^ H(g) */ /* digest1 = H(N) ^ H(g) */
if (r == 0) { if (r == 0) {
for (i = 0, j = SrpHashSize(srp->type); i < j; i++) for (i = 0; i < (word32)hashSize; i++)
digest1[i] ^= digest2[i]; digest1[i] ^= digest2[i];
} }
@@ -425,8 +430,8 @@ int wc_SrpSetParams(Srp* srp, const byte* N, word32 nSz,
SrpHashFree(&hash); SrpHashFree(&hash);
/* client proof = H( H(N) ^ H(g) | H(user) | salt) */ /* client proof = H( H(N) ^ H(g) | H(user) | salt) */
if (!r) r = SrpHashUpdate(&srp->client_proof, digest1, j); if (!r) r = SrpHashUpdate(&srp->client_proof, digest1, (word32)hashSize);
if (!r) r = SrpHashUpdate(&srp->client_proof, digest2, j); if (!r) r = SrpHashUpdate(&srp->client_proof, digest2, (word32)hashSize);
if (!r) r = SrpHashUpdate(&srp->client_proof, salt, saltSz); if (!r) r = SrpHashUpdate(&srp->client_proof, salt, saltSz);
return r; return r;
@@ -436,7 +441,7 @@ int wc_SrpSetPassword(Srp* srp, const byte* password, word32 size)
{ {
SrpHash hash; SrpHash hash;
byte digest[SRP_MAX_DIGEST_SIZE]; byte digest[SRP_MAX_DIGEST_SIZE];
word32 digestSz; int digestSz;
int r; int r;
if (!srp || !password || srp->side != SRP_CLIENT_SIDE) if (!srp || !password || srp->side != SRP_CLIENT_SIDE)
@@ -446,6 +451,8 @@ int wc_SrpSetPassword(Srp* srp, const byte* password, word32 size)
return SRP_CALL_ORDER_E; return SRP_CALL_ORDER_E;
digestSz = SrpHashSize(srp->type); digestSz = SrpHashSize(srp->type);
if (digestSz < 0)
return digestSz;
/* digest = H(username | ':' | password) */ /* digest = H(username | ':' | password) */
r = SrpHashInit(&hash, srp->type, srp->heap); r = SrpHashInit(&hash, srp->type, srp->heap);
@@ -458,12 +465,12 @@ int wc_SrpSetPassword(Srp* srp, const byte* password, word32 size)
/* digest = H(salt | H(username | ':' | password)) */ /* digest = H(salt | H(username | ':' | password)) */
if (!r) r = SrpHashInit(&hash, srp->type, srp->heap); if (!r) r = SrpHashInit(&hash, srp->type, srp->heap);
if (!r) r = SrpHashUpdate(&hash, srp->salt, srp->saltSz); if (!r) r = SrpHashUpdate(&hash, srp->salt, srp->saltSz);
if (!r) r = SrpHashUpdate(&hash, digest, digestSz); if (!r) r = SrpHashUpdate(&hash, digest, (word32)digestSz);
if (!r) r = SrpHashFinal(&hash, digest); if (!r) r = SrpHashFinal(&hash, digest);
SrpHashFree(&hash); SrpHashFree(&hash);
/* Set x (private key) */ /* Set x (private key) */
if (!r) r = mp_read_unsigned_bin(&srp->auth, digest, digestSz); if (!r) r = mp_read_unsigned_bin(&srp->auth, digest, (word32)digestSz);
ForceZero(digest, SRP_MAX_DIGEST_SIZE); ForceZero(digest, SRP_MAX_DIGEST_SIZE);
@@ -572,10 +579,15 @@ int wc_SrpGetPublic(Srp* srp, byte* pub, word32* size)
#endif #endif
word32 modulusSz; word32 modulusSz;
int r; int r;
int hashSize;
if (!srp || !pub || !size) if (!srp || !pub || !size)
return BAD_FUNC_ARG; return BAD_FUNC_ARG;
hashSize = SrpHashSize(srp->type);
if (hashSize < 0)
return hashSize;
if (mp_iszero(&srp->auth) == MP_YES) if (mp_iszero(&srp->auth) == MP_YES)
return SRP_CALL_ORDER_E; return SRP_CALL_ORDER_E;
@@ -616,7 +628,7 @@ int wc_SrpGetPublic(Srp* srp, byte* pub, word32* size)
{ {
r = mp_init_multi(i, j, 0, 0, 0, 0); r = mp_init_multi(i, j, 0, 0, 0, 0);
} }
if (!r) r = mp_read_unsigned_bin(i, srp->k,SrpHashSize(srp->type)); if (!r) r = mp_read_unsigned_bin(i, srp->k, (word32)hashSize);
if (!r) r = mp_iszero(i) == MP_YES ? SRP_BAD_KEY_E : 0; if (!r) r = mp_iszero(i) == MP_YES ? SRP_BAD_KEY_E : 0;
if (!r) r = mp_exptmod(&srp->g, &srp->priv, &srp->N, pubkey); if (!r) r = mp_exptmod(&srp->g, &srp->priv, &srp->N, pubkey);
if (!r) r = mp_mulmod(i, &srp->auth, &srp->N, j); if (!r) r = mp_mulmod(i, &srp->auth, &srp->N, j);
@@ -654,17 +666,22 @@ static int wc_SrpSetKey(Srp* srp, byte* secret, word32 size)
{ {
SrpHash hash; SrpHash hash;
byte digest[SRP_MAX_DIGEST_SIZE]; byte digest[SRP_MAX_DIGEST_SIZE];
word32 i, j, digestSz = SrpHashSize(srp->type); word32 i, j;
int digestSz;
byte counter[4]; byte counter[4];
int r = WC_NO_ERR_TRACE(BAD_FUNC_ARG); int r = WC_NO_ERR_TRACE(BAD_FUNC_ARG);
digestSz = SrpHashSize(srp->type);
if (digestSz < 0)
return digestSz;
XMEMSET(digest, 0, SRP_MAX_DIGEST_SIZE); XMEMSET(digest, 0, SRP_MAX_DIGEST_SIZE);
srp->key = (byte*)XMALLOC(2 * digestSz, srp->heap, DYNAMIC_TYPE_SRP); srp->key = (byte*)XMALLOC(2 * (word32)digestSz, srp->heap, DYNAMIC_TYPE_SRP);
if (srp->key == NULL) if (srp->key == NULL)
return MEMORY_E; return MEMORY_E;
srp->keySz = 2 * digestSz; srp->keySz = 2 * (word32)digestSz;
for (i = j = 0; j < srp->keySz; i++) { for (i = j = 0; j < srp->keySz; i++) {
counter[0] = (byte)(i >> 24); counter[0] = (byte)(i >> 24);
@@ -677,7 +694,7 @@ static int wc_SrpSetKey(Srp* srp, byte* secret, word32 size)
if (!r) r = SrpHashUpdate(&hash, counter, 4); if (!r) r = SrpHashUpdate(&hash, counter, 4);
if (!r) { if (!r) {
if (j + digestSz > srp->keySz) { if (j + (word32)digestSz > srp->keySz) {
r = SrpHashFinal(&hash, digest); r = SrpHashFinal(&hash, digest);
XMEMCPY(srp->key + j, digest, srp->keySz - j); XMEMCPY(srp->key + j, digest, srp->keySz - j);
j = srp->keySz; j = srp->keySz;
@@ -685,7 +702,7 @@ static int wc_SrpSetKey(Srp* srp, byte* secret, word32 size)
else else
{ {
r = SrpHashFinal(&hash, srp->key + j); r = SrpHashFinal(&hash, srp->key + j);
j += digestSz; j += (word32)digestSz;
} }
} }
SrpHashFree(&hash); SrpHashFree(&hash);
@@ -715,7 +732,8 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz,
mp_int u[1], s[1], temp1[1], temp2[1]; mp_int u[1], s[1], temp1[1], temp2[1];
#endif #endif
byte *secret = NULL; byte *secret = NULL;
word32 i, secretSz, digestSz; word32 i, secretSz;
int digestSz;
byte pad = 0; byte pad = 0;
int r; int r;
@@ -761,6 +779,11 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz,
goto out; goto out;
digestSz = SrpHashSize(srp->type); digestSz = SrpHashSize(srp->type);
if (digestSz < 0) {
r = digestSz;
goto out;
}
secretSz = (word32)mp_unsigned_bin_size(&srp->N); secretSz = (word32)mp_unsigned_bin_size(&srp->N);
if ((secretSz < clientPubKeySz) || (secretSz < serverPubKeySz)) { if ((secretSz < clientPubKeySz) || (secretSz < serverPubKeySz)) {
@@ -795,7 +818,7 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz,
/* set u */ /* set u */
if ((r = SrpHashFinal(hash, digest))) if ((r = SrpHashFinal(hash, digest)))
goto out; goto out;
if ((r = mp_read_unsigned_bin(u, digest, SrpHashSize(srp->type)))) if ((r = mp_read_unsigned_bin(u, digest, (word32)digestSz)))
goto out; goto out;
SrpHashFree(hash); SrpHashFree(hash);
@@ -804,7 +827,7 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz,
if (srp->side == SRP_CLIENT_SIDE) { if (srp->side == SRP_CLIENT_SIDE) {
/* temp1 = B - k * v; rejects k == 0, B == 0 and B >= N. */ /* temp1 = B - k * v; rejects k == 0, B == 0 and B >= N. */
if ((r = mp_read_unsigned_bin(temp1, srp->k, digestSz))) if ((r = mp_read_unsigned_bin(temp1, srp->k, (word32)digestSz)))
goto out; goto out;
if (mp_iszero(temp1) == MP_YES) { if (mp_iszero(temp1) == MP_YES) {
r = SRP_BAD_KEY_E; r = SRP_BAD_KEY_E;
@@ -940,11 +963,16 @@ int wc_SrpComputeKey(Srp* srp, byte* clientPubKey, word32 clientPubKeySz,
int wc_SrpGetProof(Srp* srp, byte* proof, word32* size) int wc_SrpGetProof(Srp* srp, byte* proof, word32* size)
{ {
int r; int r;
int hashSize;
if (!srp || !proof || !size) if (!srp || !proof || !size)
return BAD_FUNC_ARG; return BAD_FUNC_ARG;
if (*size < SrpHashSize(srp->type)) hashSize = SrpHashSize(srp->type);
if (hashSize < 0)
return ALGO_ID_E;
if (*size < (word32)hashSize)
return BUFFER_E; return BUFFER_E;
if ((r = SrpHashFinal(srp->side == SRP_CLIENT_SIDE if ((r = SrpHashFinal(srp->side == SRP_CLIENT_SIDE
@@ -952,7 +980,7 @@ int wc_SrpGetProof(Srp* srp, byte* proof, word32* size)
: &srp->server_proof, proof)) != 0) : &srp->server_proof, proof)) != 0)
return r; return r;
*size = SrpHashSize(srp->type); *size = (word32)hashSize;
if (srp->side == SRP_CLIENT_SIDE) { if (srp->side == SRP_CLIENT_SIDE) {
/* server proof = H( A | client proof | K) */ /* server proof = H( A | client proof | K) */
@@ -967,11 +995,16 @@ int wc_SrpVerifyPeersProof(Srp* srp, byte* proof, word32 size)
{ {
byte digest[SRP_MAX_DIGEST_SIZE]; byte digest[SRP_MAX_DIGEST_SIZE];
int r; int r;
int hashSize;
if (!srp || !proof) if (!srp || !proof)
return BAD_FUNC_ARG; return BAD_FUNC_ARG;
if (size != SrpHashSize(srp->type)) hashSize = SrpHashSize(srp->type);
if (hashSize < 0)
return ALGO_ID_E;
if (size != (word32)hashSize)
return BUFFER_E; return BUFFER_E;
r = SrpHashFinal(srp->side == SRP_CLIENT_SIDE ? &srp->server_proof r = SrpHashFinal(srp->side == SRP_CLIENT_SIDE ? &srp->server_proof