From 958fec3b453c660e31e1f3ce8a3318c90b3c5b36 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Tue, 10 Nov 2020 16:40:28 -0600 Subject: [PATCH 1/2] internal.c:ProcessPeerCerts(): fix a core.NullDereference detected by llvm9 and llvm11 scan-builds. --- src/internal.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/internal.c b/src/internal.c index 1ed9448d2..16f868bfd 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10729,6 +10729,9 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (ssl->options.tls1_3) { word16 extSz; + if (args->exts == NULL) { + ERROR_OUT(BUFFER_ERROR, exit_ppc); + } if ((args->idx - args->begin) + OPAQUE16_LEN > totalSz) { ERROR_OUT(BUFFER_ERROR, exit_ppc); } From 5fe1586688caf9039859a9aead94718cefd5577e Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 11 Nov 2020 13:04:14 -0600 Subject: [PATCH 2/2] fix 34 deadcode.DeadStores detected by llvm11 scan-build. --- src/ssl.c | 23 ++++++++++++----------- src/tls13.c | 2 +- tests/hash.c | 20 ++++++++++++-------- wolfcrypt/benchmark/benchmark.c | 8 ++++---- wolfcrypt/src/asn.c | 4 ++-- wolfcrypt/src/pkcs12.c | 20 +++++++++++--------- wolfcrypt/test/test.c | 4 ++-- 7 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index b8f3d755b..746bb5894 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -545,7 +545,7 @@ WOLFSSL* wolfSSL_new(WOLFSSL_CTX* ctx) ssl = (WOLFSSL*) XMALLOC(sizeof(WOLFSSL), ctx->heap, DYNAMIC_TYPE_SSL); if (ssl) - if ( (ret = InitSSL(ssl, ctx, 0)) < 0) { + if (InitSSL(ssl, ctx, 0) < 0) { FreeSSL(ssl, ctx->heap); ssl = 0; } @@ -6649,7 +6649,7 @@ int ProcessFile(WOLFSSL_CTX* ctx, const char* fname, int format, int type, dynamic = 1; } - if ( (ret = (int)XFREAD(myBuffer, 1, sz, file)) != sz) + if ((size_t)XFREAD(myBuffer, 1, sz, file) != (size_t)sz) ret = WOLFSSL_BAD_FILE; else { /* Try to detect type by parsing cert header and footer */ @@ -6867,7 +6867,7 @@ int wolfSSL_CertManagerVerify(WOLFSSL_CERT_MANAGER* cm, const char* fname, dynamic = 1; } - if ( (ret = (int)XFREAD(myBuffer, 1, sz, file)) != sz) + if ((size_t)XFREAD(myBuffer, 1, sz, file) != (size_t)sz) ret = WOLFSSL_BAD_FILE; else ret = wolfSSL_CertManagerVerifyBuffer(cm, myBuffer, sz, format); @@ -7292,7 +7292,7 @@ static int wolfSSL_SetTmpDH_file_wrapper(WOLFSSL_CTX* ctx, WOLFSSL* ssl, dynamic = 1; } - if ( (ret = (int)XFREAD(myBuffer, 1, sz, file)) != sz) + if ((size_t)XFREAD(myBuffer, 1, sz, file) != (size_t)sz) ret = WOLFSSL_BAD_FILE; else { if (ssl) @@ -22857,7 +22857,7 @@ int wolfSSL_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, wolfSSL_X509_free(x509); } else { - if ((ret = CopyDecodedToX509(x509, &DeCert)) != 0) { + if (CopyDecodedToX509(x509, &DeCert) != 0) { WOLFSSL_MSG("Failed to copy decoded cert"); FreeDecodedCert(&DeCert); wolfSSL_X509_free(x509); @@ -22928,7 +22928,7 @@ int wolfSSL_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, if (ParseCertRelative(&DeCert, CERT_TYPE, NO_VERIFY, NULL) != 0) { WOLFSSL_MSG("Issue with parsing certificate"); } - if ((ret = CopyDecodedToX509(*cert, &DeCert)) != 0) { + if (CopyDecodedToX509(*cert, &DeCert) != 0) { WOLFSSL_MSG("Failed to copy decoded cert"); FreeDecodedCert(&DeCert); if (pk != NULL) { @@ -22981,8 +22981,8 @@ int wolfSSL_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, XFREE(pk, heap, DYNAMIC_TYPE_PKCS); return WOLFSSL_FAILURE; } - if ((ret = wolfSSL_RSA_LoadDer_ex((*pkey)->rsa, pk, pkSz, - WOLFSSL_RSA_LOAD_PRIVATE)) != SSL_SUCCESS) { + if (wolfSSL_RSA_LoadDer_ex((*pkey)->rsa, pk, pkSz, + WOLFSSL_RSA_LOAD_PRIVATE) != SSL_SUCCESS) { WOLFSSL_MSG("issue loading RSA key"); wolfSSL_X509_free(*cert); *cert = NULL; if (ca != NULL) { @@ -37962,6 +37962,7 @@ void* wolfSSL_GetDhAgreeCtx(WOLFSSL* ssl) WOLFSSL_SUCCESS) { WOLFSSL_MSG("Unable to make DER for X509"); WOLFSSL_LEAVE("wolfSSL_X509_sign", ret); + (void)ret; ret = WOLFSSL_FAILURE; goto out; } @@ -40240,13 +40241,13 @@ WOLFSSL_DSA *wolfSSL_PEM_read_bio_DSAparams(WOLFSSL_BIO *bp, WOLFSSL_DSA **x, WOLFSSL_MSG("Not yet supporting call back or password for encrypted PEM"); } - if ((ret = PemToDer(buf, (long)bufSz, DSA_PARAM_TYPE, &pDer, NULL, NULL, - NULL)) < 0 ) { + if (PemToDer(buf, (long)bufSz, DSA_PARAM_TYPE, &pDer, NULL, NULL, + NULL) < 0 ) { WOLFSSL_MSG("Issue converting from PEM to DER"); return NULL; } - if ((ret = GetSequence(pDer->buffer, &idx, &length, pDer->length)) < 0) { + if (GetSequence(pDer->buffer, &idx, &length, pDer->length) < 0) { WOLFSSL_LEAVE("wolfSSL_PEM_read_bio_DSAparams", ret); FreeDer(&pDer); return NULL; diff --git a/src/tls13.c b/src/tls13.c index eb996a65f..e56f2fd1d 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -3355,7 +3355,7 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz, #ifdef HAVE_SESSION_TICKET /* Decode the identity. */ - if ((ret = DoClientTicket(ssl, current->identity, current->identityLen)) + if (DoClientTicket(ssl, current->identity, current->identityLen) == WOLFSSL_TICKET_RET_OK) { word32 now; int diff; diff --git a/tests/hash.c b/tests/hash.c index c99eb2908..ef0ee0c93 100644 --- a/tests/hash.c +++ b/tests/hash.c @@ -139,30 +139,34 @@ int HashTest(void) #endif #ifndef NO_SHA - if ( (ret = hmac_sha_test()) ) + if ( (ret = hmac_sha_test()) ) { printf( " HMAC-SHA test failed!\n"); - else + return ret; + } else printf( " HMAC-SHA test passed!\n"); #endif #ifdef WOLFSSL_SHA224 - if ( (ret = hmac_sha224_test()) ) + if ( (ret = hmac_sha224_test()) ) { printf( " HMAC-SHA224 test failed!\n"); - else + return ret; + } else printf( " HMAC-SHA224 test passed!\n"); #endif #ifndef NO_SHA256 - if ( (ret = hmac_sha256_test()) ) + if ( (ret = hmac_sha256_test()) ) { printf( " HMAC-SHA256 test failed!\n"); - else + return ret; + } else printf( " HMAC-SHA256 test passed!\n"); #endif #ifdef WOLFSSL_SHA384 - if ( (ret = hmac_sha384_test()) ) + if ( (ret = hmac_sha384_test()) ) { printf( " HMAC-SHA384 test failed!\n"); - else + return ret; + } else printf( " HMAC-SHA384 test passed!\n"); #endif #endif diff --git a/wolfcrypt/benchmark/benchmark.c b/wolfcrypt/benchmark/benchmark.c index 94aa4bc3d..d9f623fd4 100644 --- a/wolfcrypt/benchmark/benchmark.c +++ b/wolfcrypt/benchmark/benchmark.c @@ -4712,8 +4712,8 @@ void bench_rsa(int doAsync) /* init keys */ for (i = 0; i < BENCH_MAX_PENDING; i++) { /* setup an async context for each key */ - if ((ret = wc_InitRsaKey_ex(&rsaKey[i], HEAP_HINT, - doAsync ? devId : INVALID_DEVID)) < 0) { + if (wc_InitRsaKey_ex(&rsaKey[i], HEAP_HINT, + doAsync ? devId : INVALID_DEVID) < 0) { goto exit_bench_rsa; } @@ -4785,8 +4785,8 @@ void bench_rsa_key(int doAsync, int rsaKeySz) if (!isPending[i]) { /* if making the key is pending then just call * wc_MakeRsaKey again */ /* setup an async context for each key */ - if ((ret = wc_InitRsaKey_ex(&rsaKey[i], HEAP_HINT, - doAsync ? devId : INVALID_DEVID)) < 0) { + if (wc_InitRsaKey_ex(&rsaKey[i], HEAP_HINT, + doAsync ? devId : INVALID_DEVID) < 0) { goto exit_bench_rsa_key; } diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 226edc498..83a24b420 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -10951,7 +10951,7 @@ int wc_PemCertToDer(const char* fileName, unsigned char* derBuf, int derSz) } if (ret == 0) { - if ( (ret = (int)XFREAD(fileBuf, 1, sz, file)) != sz) { + if ((size_t)XFREAD(fileBuf, 1, sz, file) != (size_t)sz) { ret = BUFFER_E; } #ifdef WOLFSSL_PEM_TO_DER @@ -11031,7 +11031,7 @@ int wc_PemPubKeyToDer(const char* fileName, dynamic = 1; } if (ret == 0) { - if ( (ret = (int)XFREAD(fileBuf, 1, sz, file)) != sz) { + if ((size_t)XFREAD(fileBuf, 1, sz, file) != (size_t)sz) { ret = BUFFER_E; } #ifdef WOLFSSL_PEM_TO_DER diff --git a/wolfcrypt/src/pkcs12.c b/wolfcrypt/src/pkcs12.c index ce8d3b198..705c63bfb 100644 --- a/wolfcrypt/src/pkcs12.c +++ b/wolfcrypt/src/pkcs12.c @@ -227,7 +227,7 @@ static int GetSafeContent(WC_PKCS12* pkcs12, const byte* input, freeSafe(safe, pkcs12->heap); return ASN_PARSE_E; } - if ((ret = GetLength(input, &localIdx, &size, maxIdx)) <= 0) { + if (GetLength(input, &localIdx, &size, maxIdx) <= 0) { freeSafe(safe, pkcs12->heap); return ASN_PARSE_E; } @@ -250,7 +250,7 @@ static int GetSafeContent(WC_PKCS12* pkcs12, const byte* input, freeSafe(safe, pkcs12->heap); return ASN_PARSE_E; } - if ((ret = GetLength(input, &localIdx, &size, maxIdx)) <= 0) { + if (GetLength(input, &localIdx, &size, maxIdx) <= 0) { freeSafe(safe, pkcs12->heap); return ASN_PARSE_E; } @@ -366,7 +366,7 @@ static int GetSignData(WC_PKCS12* pkcs12, const byte* mem, word32* idx, * DigestAlgorithmIdentifier * Digest */ - if ((ret = GetSequence(mem, &curIdx, &size, totalSz)) <= 0) { + if (GetSequence(mem, &curIdx, &size, totalSz) <= 0) { WOLFSSL_MSG("Failed to get PKCS12 sequence"); return ASN_PARSE_E; } @@ -405,7 +405,7 @@ static int GetSignData(WC_PKCS12* pkcs12, const byte* mem, word32* idx, return ASN_PARSE_E; } - if ((ret = GetLength(mem, &curIdx, &size, totalSz)) <= 0) { + if (GetLength(mem, &curIdx, &size, totalSz) <= 0) { XFREE(mac, pkcs12->heap, DYNAMIC_TYPE_PKCS); return ASN_PARSE_E; } @@ -465,7 +465,7 @@ static int GetSignData(WC_PKCS12* pkcs12, const byte* mem, word32* idx, mac->itt = WC_PKCS12_MAC_DEFAULT; if (curIdx < totalSz) { int number = 0; - if ((ret = GetShortInt(mem, &curIdx, &number, totalSz)) >= 0) { + if (GetShortInt(mem, &curIdx, &number, totalSz) >= 0) { /* found a iteration value */ mac->itt = number; } @@ -637,7 +637,7 @@ int wc_d2i_PKCS12(const byte* der, word32 derSz, WC_PKCS12* pkcs12) } totalSz = derSz; - if ((ret = GetSequence(der, &idx, &size, totalSz)) <= 0) { + if (GetSequence(der, &idx, &size, totalSz) <= 0) { WOLFSSL_MSG("Failed to get PKCS12 sequence"); return ASN_PARSE_E; } @@ -1008,6 +1008,7 @@ int wc_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, pkcs12->safe->dataSz, (byte*)psw, pswSz)) != 0) { WOLFSSL_MSG("PKCS12 Bad MAC on verify"); WOLFSSL_LEAVE("wc_PKCS12_parse verify ", ret); + (void)ret; return MAC_CMP_FAILED_E; } } @@ -1099,7 +1100,7 @@ int wc_PKCS12_parse(WC_PKCS12* pkcs12, const char* psw, if (tag != (ASN_CONSTRUCTED | ASN_CONTEXT_SPECIFIC)) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } - if ((ret = GetLength(data, &idx, &size, ci->dataSz)) <= 0) { + if (GetLength(data, &idx, &size, ci->dataSz) <= 0) { ERROR_OUT(ASN_PARSE_E, exit_pk12par); } @@ -2245,7 +2246,7 @@ WC_PKCS12* wc_PKCS12_create(char* pass, word32 passSz, char* name, WOLFSSL_ENTER("wc_PKCS12_create()"); - if ((ret = wc_InitRng_ex(&rng, heap, INVALID_DEVID)) != 0) { + if (wc_InitRng_ex(&rng, heap, INVALID_DEVID) != 0) { return NULL; } @@ -2259,6 +2260,7 @@ WC_PKCS12* wc_PKCS12_create(char* pass, word32 passSz, char* name, wc_PKCS12_free(pkcs12); wc_FreeRng(&rng); WOLFSSL_LEAVE("wc_PKCS12_create", ret); + (void)ret; return NULL; } @@ -2340,7 +2342,7 @@ WC_PKCS12* wc_PKCS12_create(char* pass, word32 passSz, char* name, return NULL; } - if ((ret = wc_RNG_GenerateBlock(&rng, mac->salt, mac->saltSz)) != 0) { + if (wc_RNG_GenerateBlock(&rng, mac->salt, mac->saltSz) != 0) { WOLFSSL_MSG("Error generating random salt"); wc_PKCS12_free(pkcs12); wc_FreeRng(&rng); diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 566c3adc2..2dff5521d 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -18450,7 +18450,7 @@ static int pkcs12_test(void) if (ret < 0) return -9100; - if ( (ret = XMEMCMP(derived, verify, kLen)) != 0) + if (XMEMCMP(derived, verify, kLen) != 0) return -9101; iterations = 1000; @@ -18464,7 +18464,7 @@ static int pkcs12_test(void) if (ret < 0) return -9103; - if ( (ret = XMEMCMP(derived, verify2, 24)) != 0) + if (XMEMCMP(derived, verify2, 24) != 0) return -9104; return 0;