From fbdb064a4beecc8541c2050fb629524395c34a20 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 11:24:42 -0600 Subject: [PATCH 1/6] coverity issue 394701 possible derefernce before null check --- wolfcrypt/src/rsa.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index c758ab27a0..5a9df20023 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -673,13 +673,17 @@ static int _ifc_pairwise_consistency_test(RsaKey* key, WC_RNG* rng) int wc_CheckRsaKey(RsaKey* key) { - DECL_MP_INT_SIZE_DYN(tmp, mp_bitsused(&key->n), RSA_MAX_SIZE); #ifdef WOLFSSL_SMALL_STACK WC_RNG *rng = NULL; #else WC_RNG rng[1]; #endif int ret = 0; + DECL_MP_INT_SIZE_DYN(tmp, (key)? mp_bitsused(&key->n) : 0, RSA_MAX_SIZE); + + if (key == NULL) { + return BAD_FUNC_ARG; + } #ifdef WOLFSSL_CAAM /* can not perform these checks on an encrypted key */ @@ -711,11 +715,6 @@ int wc_CheckRsaKey(RsaKey* key) ret = MP_INIT_E; } - if (ret == 0) { - if (key == NULL) - ret = BAD_FUNC_ARG; - } - if (ret == 0) ret = _ifc_pairwise_consistency_test(key, rng); From 50a7243486d5e92704c89bb5279a9b7e0a95bf0e Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 11:53:19 -0600 Subject: [PATCH 2/6] fix for coverity issue 394670 possible overflow --- src/ssl_load.c | 2 +- wolfcrypt/src/dh.c | 8 ++++++++ wolfcrypt/test/test.c | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/ssl_load.c b/src/ssl_load.c index 30245bfb6b..06f8b84f91 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -5563,7 +5563,7 @@ long wolfSSL_CTX_set_tmp_dh(WOLFSSL_CTX* ctx, WOLFSSL_DH* dh) ret = wolfssl_ctx_set_tmp_dh(ctx, p, pSz, g, gSz); } - if (ret != 1) { + if (ret != 1 && ctx != NULL) { /* Free the allocated buffers if not assigned into SSL. */ XFREE(p, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(g, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY); diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index 041915998a..28ed1978d7 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -2940,6 +2940,14 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh) if (ret == 0) { /* modulus size in bytes */ modSz /= WOLFSSL_BIT_SIZE; + + if ((word32)modSz < groupSz) { + WOLFSSL_MSG("DH modSz was too small"); + ret = BAD_FUNC_ARG; + } + } + + if (ret == 0) { bufSz = (word32)modSz - groupSz; /* allocate ram */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index ebd0075a0e..f252366768 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -22272,6 +22272,11 @@ static wc_test_ret_t dh_generate_test(WC_RNG *rng) if (ret != 0) ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test); + /* should fail since modSz is 16 and group size is 20 */ + ret = wc_DhGenerateParams(rng, 128, smallKey); + if (ret == 0) + ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test); + ret = wc_DhGenerateParams(rng, 2056, smallKey); if (ret != 0) ERROR_OUT(WC_TEST_RET_ENC_EC(ret), exit_gen_test); From 25d52dde3f7d3308b48d25da37d3ded1ddd3dc2e Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 11:56:45 -0600 Subject: [PATCH 3/6] fix coverity issue 367842 possible null dereference --- src/ssl_load.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl_load.c b/src/ssl_load.c index 06f8b84f91..9c30e84383 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -5264,7 +5264,7 @@ int wolfSSL_SetTmpDH(WOLFSSL* ssl, const unsigned char* p, int pSz, ret = wolfssl_set_tmp_dh(ssl, pAlloc, pSz, gAlloc, gSz); } - if (ret != 1) { + if (ret != 1 && ssl != NULL) { /* Free the allocated buffers if not assigned into SSL. */ XFREE(pAlloc, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(gAlloc, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); From c880fcf822e962bc4690e28dd94989bdbdcf62c1 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 12:07:42 -0600 Subject: [PATCH 4/6] add check on padSz return, coverity issue 394711 --- wolfcrypt/src/pkcs7.c | 2 ++ wolfcrypt/test/test.c | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index a2c838b8ce..acf7ef8f22 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -8634,6 +8634,8 @@ int wc_PKCS7_PadData(byte* in, word32 inSz, byte* out, word32 outSz, return BAD_FUNC_ARG; padSz = wc_PKCS7_GetPadSize(inSz, blockSz); + if (padSz < 0) + return padSz; if (outSz < (inSz + padSz)) return BAD_FUNC_ARG; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index f252366768..da560e20cd 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -49471,6 +49471,11 @@ static wc_test_ret_t pkcs7signed_run_vectors( XMEMSET(out, 0, outSz); + /* test inner pad size error with block size being 0 */ + ret = wc_PKCS7_PadData((byte*)data, sizeof(data), out, outSz, 0); + if (ret > 0) + ERROR_OUT(-1, out); + ret = wc_PKCS7_PadData((byte*)data, sizeof(data), out, outSz, 16); if (ret < 0) ERROR_OUT(WC_TEST_RET_ENC_EC(ret), out); From d4cf93c2cfd8076b00ac592375645f63bae7b158 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 13:17:53 -0600 Subject: [PATCH 5/6] avoid overflow if clamping a digit with used size 0 --- wolfssl/wolfcrypt/sp_int.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/wolfssl/wolfcrypt/sp_int.h b/wolfssl/wolfcrypt/sp_int.h index 514b7110fd..ba16895617 100644 --- a/wolfssl/wolfcrypt/sp_int.h +++ b/wolfssl/wolfcrypt/sp_int.h @@ -695,9 +695,11 @@ typedef struct sp_ecc_ctx { #define sp_clamp(a) \ do { \ int ii; \ - for (ii = (int)(a)->used - 1; ii >= 0 && (a)->dp[ii] == 0; ii--) { \ + if ((a)->used > 0) { \ + for (ii = (int)(a)->used - 1; ii >= 0 && (a)->dp[ii] == 0; ii--) { \ + } \ + (a)->used = (unsigned int)ii + 1; \ } \ - (a)->used = (unsigned int)ii + 1; \ } while (0) /* Check the compiled and linked math implementation are the same. From f8eb0c3776f33ca3201ab43263a5d81ffa6911e8 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 5 Jul 2024 14:06:19 -0600 Subject: [PATCH 6/6] fix for coverity issue 394676 possible null dereference --- src/ssl_load.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl_load.c b/src/ssl_load.c index 9c30e84383..2441d485e6 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -5496,7 +5496,7 @@ long wolfSSL_set_tmp_dh(WOLFSSL *ssl, WOLFSSL_DH *dh) ret = wolfssl_set_tmp_dh(ssl, p, pSz, g, gSz); } - if (ret != 1) { + if (ret != 1 && ssl != NULL) { /* Free the allocated buffers if not assigned into SSL. */ XFREE(p, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); XFREE(g, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);