From 45503972f8c10f132daf8c255c844e44dad9941a Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Fri, 19 May 2023 12:17:41 +1000 Subject: [PATCH] scan-build fixes sp_mulmod - scan-build getting confused with size of result - don't check result size as checked already - split out implementation of sp_mulmod from check StoreEccKey - ensure pubKey is not NULL even though all uses will not be GetCertKey - ensure source is not NULL - cert->source may be NULL in incorrect usages of APIs --- wolfcrypt/src/asn.c | 15 ++++++++++ wolfcrypt/src/sp_int.c | 62 ++++++++++++++++++++++++++++-------------- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b9398462a..67f91768c 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11451,6 +11451,7 @@ enum { * @param [in] pubKey Buffer holding encoded public key. * @param [in] pubKeyLen Length of encoded public key in bytes. * @return 0 on success. + * @return BAD_FUNC_ARG when pubKey is NULL. * @return ASN_PARSE_E when BER encoded data does not match ASN.1 items or * is invalid. * @return BUFFER_E when data in buffer is too small. @@ -11470,6 +11471,10 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx, byte tag; int length; + if (pubKey == NULL) { + return BAD_FUNC_ARG; + } + localIdx = *srcIdx; if (GetASNTag(source, &localIdx, &tag, maxIdx) < 0) return ASN_PARSE_E; @@ -11527,6 +11532,11 @@ static int StoreEccKey(DecodedCert* cert, const byte* source, word32* srcIdx, DECL_ASNGETDATA(dataASN, eccCertKeyASN_Length); byte* publicKey; + /* Validate parameters. */ + if (pubKey == NULL) { + ret = BAD_FUNC_ARG; + } + /* Clear dynamic data and check OID is a curve. */ CALLOC_ASNGETDATA(dataASN, eccCertKeyASN_Length, ret, cert->heap); if (ret == 0) { @@ -11695,6 +11705,11 @@ static int GetCertKey(DecodedCert* cert, const byte* source, word32* inOutIdx, int ret = 0; int length; + /* Validate paramaters. */ + if (source == NULL) { + return ASN_PARSE_E; + } + #ifndef WOLFSSL_ASN_TEMPLATE if (GetSequence(source, &srcIdx, &length, maxIdx) < 0) #else diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index df16fa6eb..58f853046 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -11696,7 +11696,7 @@ int sp_mul(const sp_int* a, const sp_int* b, sp_int* r) * @return MP_OKAY on success. * @return MP_MEM when dynamic memory allocation fails. */ -static int _sp_mulmod(const sp_int* a, const sp_int* b, const sp_int* m, +static int _sp_mulmod_tmp(const sp_int* a, const sp_int* b, const sp_int* m, sp_int* r) { int err = MP_OKAY; @@ -11722,6 +11722,39 @@ static int _sp_mulmod(const sp_int* a, const sp_int* b, const sp_int* m, return err; } +/* Multiply a by b mod m and store in r: r = (a * b) mod m + * + * @param [in] a SP integer to multiply. + * @param [in] b SP integer to multiply. + * @param [in] m SP integer that is the modulus. + * @param [out] r SP integer result. + * + * @return MP_OKAY on success. + * @return MP_MEM when dynamic memory allocation fails. + */ +static int _sp_mulmod(const sp_int* a, const sp_int* b, const sp_int* m, + sp_int* r) +{ + int err = MP_OKAY; + + /* Use r as intermediate result if not same as pointer m which is needed + * after first intermediate result. + */ + if (r != m) { + /* Multiply and reduce. */ + err = sp_mul(a, b, r); + if (err == MP_OKAY) { + err = sp_mod(r, m, r); + } + } + else { + /* Do operation using temporary. */ + _sp_mulmod_tmp(a, b, m, r); + } + + return err; +} + /* Multiply a by b mod m and store in r: r = (a * b) mod m * * @param [in] a SP integer to multiply. @@ -11755,19 +11788,8 @@ int sp_mulmod(const sp_int* a, const sp_int* b, const sp_int* m, sp_int* r) } #endif - /* Use r as intermediate result if not same as pointer m which is needed - * after first intermediate result. - */ - if ((err == MP_OKAY) && (r != m)) { - /* Multiply and reduce. */ - err = sp_mul(a, b, r); - if (err == MP_OKAY) { - err = sp_mod(r, m, r); - } - } - else if (err == MP_OKAY) { - /* Do operation using temporary. */ - _sp_mulmod(a, b, m, r); + if (err == MP_OKAY) { + err = _sp_mulmod(a, b, m, r); } #if 0 @@ -12562,7 +12584,7 @@ static int _sp_exptmod_ex(const sp_int* b, const sp_int* e, int bits, /* 4.4 s = s | y */ s |= y; /* 4.5. t[j] = t[j] * b */ - err = sp_mulmod(t[j], b, m, t[j]); + err = _sp_mulmod(t[j], b, m, t[j]); } #else /* 4.1. t[s] = t[s] ^ 2 */ @@ -12585,7 +12607,7 @@ static int _sp_exptmod_ex(const sp_int* b, const sp_int* e, int bits, _sp_copy((sp_int*)(((size_t)t[0] & sp_off_on_addr[j^1]) + ((size_t)t[1] & sp_off_on_addr[j ])), t[2]); - err = sp_mulmod(t[2], b, m, t[2]); + err = _sp_mulmod(t[2], b, m, t[2]); _sp_copy(t[2], (sp_int*)(((size_t)t[0] & sp_off_on_addr[j^1]) + ((size_t)t[1] & sp_off_on_addr[j ]))); @@ -12682,7 +12704,7 @@ static int _sp_exptmod_mont_ex(const sp_int* b, const sp_int* e, int bits, */ err = sp_mont_norm(t[1], m); if (err == MP_OKAY) { - err = sp_mulmod(t[0], t[1], m, t[0]); + err = _sp_mulmod(t[0], t[1], m, t[0]); } if (err == MP_OKAY) { /* 4. t[1] = t[0] @@ -12860,7 +12882,7 @@ static int _sp_exptmod_mont_ex(const sp_int* b, const sp_int* e, int bits, err = sp_mont_norm(t[0], m); if (err == MP_OKAY) { /* 3. t[1] = ToMont(t[1]) */ - err = sp_mulmod(t[1], t[0], m, t[1]); + err = _sp_mulmod(t[1], t[0], m, t[1]); } /* 4. For i in 2..(2 ^ w) - 1 */ @@ -13556,7 +13578,7 @@ static int _sp_exptmod_nct(const sp_int* b, const sp_int* e, const sp_int* m, err = sp_mont_norm(t[0], m); if (err == MP_OKAY) { /* 2. Convert base to Montgomery form. */ - err = sp_mulmod(bm, t[0], m, bm); + err = _sp_mulmod(bm, t[0], m, bm); } if (err == MP_OKAY) { /* Copy Montgomery form of base into first element of table. */ @@ -13807,7 +13829,7 @@ static int _sp_exptmod_nct(const sp_int* b, const sp_int* e, const sp_int* m, err = sp_mont_norm(t[1], m); if (err == MP_OKAY) { /* 1. Convert base to Montgomery form. */ - err = sp_mulmod(t[0], t[1], m, t[0]); + err = _sp_mulmod(t[0], t[1], m, t[0]); } if (err == MP_OKAY) { /* 2. Result starts as Montgomery form of base (assuming e > 0). */