From 293d7e12414f18b38985b9a1b841b1d72530b419 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 13:31:09 -0700 Subject: [PATCH 01/11] Fix for report of `Use of memory after it is freed`. Force the `dataIsAlloc` set to 0. --- wolfcrypt/src/rsa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index 7866d9ed4..734d47486 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -3278,6 +3278,7 @@ static int RsaPrivateDecryptEx(const byte* in, word32 inLen, byte* out, XMEMCPY(key->data, in, inLen); } else { + key->dataIsAlloc = 0; key->data = out; } #endif From 5a75e0f6c6089b6b56c720118caae6e91b47644e Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 13:31:39 -0700 Subject: [PATCH 02/11] Fix for MCAPI `CRYPT_AES_CTX` size with `./configure --enable-pkcallbacks --enable-mcapi --enable-ecc --enable-sha512 --with-libz --enable-opensslextra`. --- mcapi/crypto.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcapi/crypto.h b/mcapi/crypto.h index 7e4794964..cb3aae402 100644 --- a/mcapi/crypto.h +++ b/mcapi/crypto.h @@ -173,7 +173,7 @@ enum { typedef struct CRYPT_AES_CTX { /* big enough to hold internal, but check on init */ #ifdef WOLF_PRIVATE_KEY_ID - int holder[104]; + int holder[108]; #else int holder[90]; #endif From a6a89d3316bbe638f210240f853b1461a3a8ee95 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 13:55:19 -0700 Subject: [PATCH 03/11] Fix for integer.c `s_mp_add` output to make sure it grows if not set. --- wolfcrypt/src/integer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/integer.c b/wolfcrypt/src/integer.c index e52152771..c75eab1e3 100644 --- a/wolfcrypt/src/integer.c +++ b/wolfcrypt/src/integer.c @@ -1713,7 +1713,7 @@ int s_mp_add (mp_int * a, mp_int * b, mp_int * c) } /* init result */ - if (c->alloc < max_ab + 1) { + if (c->dp == NULL || c->alloc < max_ab + 1) { if ((res = mp_grow (c, max_ab + 1)) != MP_OKAY) { return res; } @@ -1757,7 +1757,7 @@ int s_mp_add (mp_int * a, mp_int * b, mp_int * c) if (min_ab != max_ab) { for (; i < max_ab; i++) { /* T[i] = X[i] + U */ - *tmpc = x->dp[i] + u; // NOLINT(clang-analyzer-core.NullDereference) /* clang-tidy 13 false positive */ + *tmpc = x->dp[i] + u; /* U = carry bit of T[i] */ u = *tmpc >> ((mp_digit)DIGIT_BIT); From c41b1b1b9bfe2abcd8afcff74f68d3bd0e78db50 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 14:16:04 -0700 Subject: [PATCH 04/11] Fix to ensure args->dCert is set for `ProcessPeerCertParse`. --- src/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index ba1b0f34e..bc7f1ab67 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11696,7 +11696,7 @@ static int ProcessPeerCertParse(WOLFSSL* ssl, ProcPeerCertArgs* args, int sigRet = 0; #endif - if (ssl == NULL || args == NULL) + if (ssl == NULL || args == NULL || args->dCert == NULL) return BAD_FUNC_ARG; /* check to make sure certificate index is valid */ From 3755b88a02511b7da9242ec5864d04c0dd519241 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 14:27:26 -0700 Subject: [PATCH 05/11] Fix `InitX509Name` to set `dynamicName` on init. --- src/internal.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index bc7f1ab67..750baa064 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3871,13 +3871,11 @@ static enum wc_HashType HashAlgoToType(int hashAlgo) void InitX509Name(WOLFSSL_X509_NAME* name, int dynamicFlag, void* heap) { - (void)dynamicFlag; - (void)heap; - if (name != NULL) { XMEMSET(name, 0, sizeof(WOLFSSL_X509_NAME)); name->name = name->staticName; name->heap = heap; + name->dynamicName = dynamicFlag; } } From 84a33183a6f61f211711b3159c71441ec059ae20 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 21 Apr 2022 14:44:23 -0700 Subject: [PATCH 06/11] Various scan-build fixes. --- examples/client/client.c | 3 +-- src/internal.c | 16 ++++++++++------ src/ssl.c | 1 - wolfcrypt/test/test.c | 4 ++-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 3e79517d8..3fc2b715a 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -2069,7 +2069,6 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) (void)resumeScr; (void)ourKey; (void)ourCert; - (void)customVerifyCert; (void)verifyCert; (void)useClientCert; (void)disableCRL; @@ -3898,7 +3897,6 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) if (ret == WOLFSSL_SUCCESS) { printf("NON-BLOCKING RENEGOTIATION SUCCESSFUL\n"); - err = 0; } } if (ret != WOLFSSL_SUCCESS) { @@ -4342,6 +4340,7 @@ exit: (void) ourCert; (void) ourKey; (void) useVerifyCb; + (void) customVerifyCert; #if !defined(WOLFSSL_TIRTOS) return 0; diff --git a/src/internal.c b/src/internal.c index 750baa064..040439738 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10741,12 +10741,16 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) #endif } - /* store cert for potential retrieval */ - if (AllocDer(&x509->derCert, dCert->maxIdx, CERT_TYPE, x509->heap) == 0) { - XMEMCPY(x509->derCert->buffer, dCert->source, dCert->maxIdx); - } - else { - ret = MEMORY_E; + /* if der contains original source buffer then store for potential + * retrieval */ + if (dCert->source != NULL && dCert->maxIdx > 0) { + if (AllocDer(&x509->derCert, dCert->maxIdx, CERT_TYPE, x509->heap) + == 0) { + XMEMCPY(x509->derCert->buffer, dCert->source, dCert->maxIdx); + } + else { + ret = MEMORY_E; + } } x509->altNames = dCert->altNames; diff --git a/src/ssl.c b/src/ssl.c index 594a391e4..f9c80e51e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -7600,7 +7600,6 @@ int wolfSSL_CTX_load_verify_locations_ex(WOLFSSL_CTX* ctx, const char* file, if (ret != WOLFSSL_SUCCESS) { WOLFSSL_MSG("wolfSSL_CTX_trust_peer_cert error. Ignoring" "this error."); - ret = WOLFSSL_SUCCESS; } #endif successCount++; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 10e40700c..72b736bb0 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -16674,11 +16674,11 @@ WOLFSSL_TEST_SUBROUTINE int dh_test(void) #ifdef HAVE_FFDHE_4096 #ifdef HAVE_PUBLIC_FFDHE ret = dh_ffdhe_test(&rng, wc_Dh_ffdhe4096_Get()); - if (ret != 0) - ERROR_OUT(-8128, done); #else ret = dh_ffdhe_test(&rng, WC_FFDHE_4096); #endif + if (ret != 0) + ERROR_OUT(-8128, done); #endif #endif /* !WC_NO_RNG */ #endif /* HAVE_FIPS_VERSION == 2 && !WOLFSSL_SP_ARM64_ASM */ From ea2841fa7a4c9ab2164cbe0f2e79bff251d5a541 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 22 Apr 2022 08:48:12 -0700 Subject: [PATCH 07/11] Make sure ASN1 isDynamic is always set to 0. SK Cipher doesn't have free (data is contained in the SK). --- src/ssl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index f9c80e51e..cbd456d83 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -21462,6 +21462,7 @@ WOLFSSL_ASN1_INTEGER* wolfSSL_ASN1_INTEGER_new(void) XMEMSET(a, 0, sizeof(WOLFSSL_ASN1_INTEGER)); a->data = a->intData; + a->isDynamic = 0; a->dataMax = WOLFSSL_ASN1_INTEGER_MAX; a->length = 0; return a; @@ -24866,9 +24867,7 @@ void wolfSSL_sk_pop_free(WOLF_STACK_OF(WOLFSSL_ASN1_OBJECT)* sk, WOLFSSL_STACK* next = sk->next; if (func != NULL) { - if (sk->type == STACK_TYPE_CIPHER) - func(&sk->data.cipher); - else + if (sk->type != STACK_TYPE_CIPHER) func(sk->data.generic); } XFREE(sk, NULL, DYNAMIC_TYPE_OPENSSL); @@ -25982,6 +25981,7 @@ WOLFSSL_ASN1_INTEGER* wolfSSL_BN_to_ASN1_INTEGER(const WOLFSSL_BIGNUM *bn, WOLFS else { XMEMSET(a->intData, 0, sizeof(a->intData)); a->data = a->intData; + a->isDynamic = 0; } /* populate data */ @@ -40941,10 +40941,10 @@ int wolfSSL_a2i_ASN1_INTEGER(WOLFSSL_BIO *bio, WOLFSSL_ASN1_INTEGER *asn1, /* Reset asn1 */ if (asn1->isDynamic && asn1->data) { XFREE(asn1->data, NULL, DYNAMIC_TYPE_OPENSSL); - asn1->isDynamic = 0; } XMEMSET(asn1->intData, 0, WOLFSSL_ASN1_INTEGER_MAX); asn1->data = asn1->intData; + asn1->isDynamic = 0; asn1->length = 0; asn1->negative = 0; asn1->type = V_ASN1_INTEGER; From 74cd2fd910b2449b002b3b3c1523f9287272d47f Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 22 Apr 2022 09:15:52 -0700 Subject: [PATCH 08/11] Fix for integer.c possible uses of mp_int input with DP NULL. --- wolfcrypt/src/integer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/wolfcrypt/src/integer.c b/wolfcrypt/src/integer.c index c75eab1e3..54c8b1eca 100644 --- a/wolfcrypt/src/integer.c +++ b/wolfcrypt/src/integer.c @@ -4378,6 +4378,10 @@ int mp_add_d (mp_int* a, mp_digit b, mp_int* c) // NOLINT(misc-no-recursion) /* destination alias */ tmpc = c->dp; + if (tmpa == NULL || tmpc == NULL) { + return MP_MEM; + } + /* if a is positive */ if (a->sign == MP_ZPOS) { /* add digit, after this we're propagating @@ -4462,6 +4466,10 @@ int mp_sub_d (mp_int * a, mp_digit b, mp_int * c) // NOLINT(misc-no-recursion) tmpa = a->dp; tmpc = c->dp; + if (tmpa == NULL || tmpc == NULL) { + return MP_MEM; + } + /* if a <= b simply fix the single digit */ if ((a->used == 1 && a->dp[0] <= b) || a->used == 0) { if (a->used == 1) { From e9b80e53facd773d2d3b236d61618baf15cc3618 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 22 Apr 2022 10:09:47 -0700 Subject: [PATCH 09/11] Fix issue with `InitX509Name`. --- src/internal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 040439738..92a9de033 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3871,11 +3871,13 @@ static enum wc_HashType HashAlgoToType(int hashAlgo) void InitX509Name(WOLFSSL_X509_NAME* name, int dynamicFlag, void* heap) { + (void)dynamicFlag; + if (name != NULL) { XMEMSET(name, 0, sizeof(WOLFSSL_X509_NAME)); name->name = name->staticName; name->heap = heap; - name->dynamicName = dynamicFlag; + name->dynamicName = 0; } } From 4ecf3545d7b720d97bb673317e767374d67e29c5 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 22 Apr 2022 16:07:24 -0700 Subject: [PATCH 10/11] Improve scan-build fix for `ProcessPeerCertParse` checking of empty `dCert`. With `WOLFSSL_SMALL_CERT_VERIFY` it is NULL. --- src/internal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 92a9de033..8005c2699 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11700,8 +11700,13 @@ static int ProcessPeerCertParse(WOLFSSL* ssl, ProcPeerCertArgs* args, int sigRet = 0; #endif - if (ssl == NULL || args == NULL || args->dCert == NULL) + if (ssl == NULL || args == NULL + #ifndef WOLFSSL_SMALL_CERT_VERIFY + || args->dCert == NULL + #endif + ) { return BAD_FUNC_ARG; + } /* check to make sure certificate index is valid */ if (args->certIdx > args->count) From 34d541109dfe51330f6f65aef63fc97d9f3924d5 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 25 Apr 2022 09:55:36 -0700 Subject: [PATCH 11/11] Additional scan-build warning fixes. --- mcapi/mcapi_test.c | 22 ++++++++++++---------- wolfcrypt/src/integer.c | 4 ++-- wolfcrypt/test/test.c | 1 + 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/mcapi/mcapi_test.c b/mcapi/mcapi_test.c index 57c2a1468..6589dfe39 100644 --- a/mcapi/mcapi_test.c +++ b/mcapi/mcapi_test.c @@ -562,14 +562,13 @@ static int check_compress(void) printf("compress dynamic ret failed\n"); return -1; } + outSz = ret1; - if (memcmp(cBuffer, dBuffer, ret1) != 0) { + if (memcmp(cBuffer, dBuffer, outSz) != 0) { printf("compress dynamic cmp failed\n"); return -1; } - outSz = ret1; - ret1 = CRYPT_HUFFMAN_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); if (memcmp(dBuffer, text, inSz) != 0) { @@ -578,9 +577,11 @@ static int check_compress(void) } memset(dBuffer, 0, sizeof(dBuffer)); + ret2 = wc_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); - ret1 = wc_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); - + if (ret1 != ret2 || ret2 < 0) { + printf("decompress dynamic ret failed\n"); + } if (memcmp(dBuffer, text, inSz) != 0) { printf("decompress dynamic cmp failed\n"); return -1; @@ -597,13 +598,13 @@ static int check_compress(void) printf("compress static ret failed\n"); return -1; } + outSz = ret1; - if (memcmp(cBuffer, dBuffer, ret1) != 0) { + if (memcmp(cBuffer, dBuffer, outSz) != 0) { printf("compress static cmp failed\n"); return -1; } - outSz = ret1; ret1 = CRYPT_HUFFMAN_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); @@ -613,9 +614,10 @@ static int check_compress(void) } memset(dBuffer, 0, sizeof(dBuffer)); - - ret1 = wc_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); - + ret2 = wc_DeCompress(dBuffer, sizeof(dBuffer), cBuffer, outSz); + if (ret1 != ret2 || ret2 < 0) { + printf("decompress static ret failed\n"); + } if (memcmp(dBuffer, text, inSz) != 0) { printf("decompress static cmp failed\n"); return -1; diff --git a/wolfcrypt/src/integer.c b/wolfcrypt/src/integer.c index 54c8b1eca..036b53298 100644 --- a/wolfcrypt/src/integer.c +++ b/wolfcrypt/src/integer.c @@ -511,7 +511,7 @@ void mp_zero (mp_int * a) a->used = 0; tmp = a->dp; - for (n = 0; n < a->alloc; n++) { + for (n = 0; tmp != NULL && n < a->alloc; n++) { *tmp++ = 0; } } @@ -2962,7 +2962,7 @@ int mp_mul_d (mp_int * a, mp_digit b, mp_int * c) int ix, res, olduse; /* make sure c is big enough to hold a*b */ - if (c->alloc < a->used + 1) { + if (c->dp == NULL || c->alloc < a->used + 1) { if ((res = mp_grow (c, a->used + 1)) != MP_OKAY) { return res; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 72b736bb0..1a702fbcd 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -31862,6 +31862,7 @@ WOLFSSL_TEST_SUBROUTINE int compress_test(void) if ((ret = wc_DeCompress(d, dSz, c, cSz)) != (int)dSz) { ERROR_OUT(-12102, exit); } + dSz = (word32)ret; if (XMEMCMP(d, sample_text, dSz) != 0) { ERROR_OUT(-12103, exit);