Merge pull request #7714 from JacobBarthelmeh/coverity

Coverity issues reported
wolfSSL · Jul 5, 2024 · d5016d4 · d5016d4
2 parents 7ef424b + f8eb0c3
commit d5016d4
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 11 deletions.
diff --git 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);
@@ -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);
@@ -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
@@ -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/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/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);
 

diff --git 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);
@@ -49466,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);

diff --git 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.