Merge pull request wolfSSL#7733 from SparkiDev/coverity_3

Coverity fixes
gojimmypi · Jul 10, 2024 · 2046687 · 2046687
2 parents 9bd0985 + fea7a89
commit 2046687
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 20 deletions.
diff --git a/src/pk.c b/src/pk.c
@@ -376,7 +376,7 @@ int EncryptDerKey(byte *der, int *derSz, const EVP_CIPHER* cipher,
             DYNAMIC_TYPE_ENCRYPTEDINFO);
         if (info == NULL) {
             WOLFSSL_MSG("malloc failed");
-            ret = 0;
+            ret = MEMORY_E;
         }
     }
     #endif
@@ -417,7 +417,8 @@ int EncryptDerKey(byte *der, int *derSz, const EVP_CIPHER* cipher,
         (*derSz) += (int)paddingSz;
 
         /* Encrypt DER buffer. */
-        ret = wc_BufferKeyEncrypt(info, der, (word32)*derSz, passwd, passwdSz, WC_MD5);
+        ret = wc_BufferKeyEncrypt(info, der, (word32)*derSz, passwd, passwdSz,
+            WC_MD5);
         if (ret != 0) {
             WOLFSSL_MSG("encrypt key failed");
         }
@@ -3273,6 +3274,7 @@ static int wolfssl_rsa_generate_key_native(WOLFSSL_RSA* rsa, int bits,
 #endif
     int initTmpRng = 0;
     WC_RNG* rng = NULL;
+    long en;
 #endif
 
     (void)cb;
@@ -3286,10 +3288,12 @@ static int wolfssl_rsa_generate_key_native(WOLFSSL_RSA* rsa, int bits,
         /* Something went wrong so return memory error. */
         ret = MEMORY_E;
     }
+    if ((ret == 0) && ((en = (long)wolfSSL_BN_get_word(e)) <= 0)) {
+        ret = BAD_FUNC_ARG;
+    }
     if (ret == 0) {
         /* Generate an RSA key. */
-        ret = wc_MakeRsaKey((RsaKey*)rsa->internal, bits,
-            (long)wolfSSL_BN_get_word(e), rng);
+        ret = wc_MakeRsaKey((RsaKey*)rsa->internal, bits, en, rng);
         if (ret != MP_OKAY) {
             WOLFSSL_ERROR_MSG("wc_MakeRsaKey failed");
         }

diff --git a/src/ssl_load.c b/src/ssl_load.c
@@ -1227,8 +1227,13 @@ static int ProcessBufferPrivPkcs8Dec(EncryptedInfo* info, DerBuffer* der,
         der->length = (word32)ret;
     }
 
-    /* Ensure password is zeroized. */
-    ForceZero(password, (word32)passwordSz);
+#ifdef WOLFSSL_SMALL_STACK
+    if (password != NULL)
+#endif
+    {
+        /* Ensure password is zeroized. */
+        ForceZero(password, (word32)passwordSz);
+    }
 #ifdef WOLFSSL_SMALL_STACK
     /* Dispose of password memory. */
     XFREE(password, heap, DYNAMIC_TYPE_STRING);
@@ -5563,7 +5568,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 && ctx != NULL) {
+    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/tests/api.c b/tests/api.c
@@ -65626,7 +65626,7 @@ static int test_RsaSigFailure_cm(void)
     size_t cert_sz = 0;
 
     ExpectIntEQ(load_file(server_cert, &cert_buf, &cert_sz), 0);
-    if (cert_buf != NULL) {
+    if ((cert_buf != NULL) && (cert_sz > 0)) {
         /* corrupt DER - invert last byte, which is signature */
         cert_buf[cert_sz-1] = ~cert_buf[cert_sz-1];
         /* test bad cert */
@@ -77620,6 +77620,7 @@ static int send_new_session_ticket(WOLFSSL *ssl, byte nonceLength, byte filler)
 
     sz = BuildTls13Message(ssl, buf, 2048, buf+5, idx - 5,
         handshake, 0, 0, 0);
+    AssertIntGT(sz, 0);
     test_ctx = (struct test_memio_ctx*)wolfSSL_GetIOWriteCtx(ssl);
     AssertNotNull(test_ctx);
     ret = test_memio_write_cb(ssl, (char*)buf, sz, test_ctx);
@@ -77718,8 +77719,16 @@ static int test_ticket_nonce_malloc(void)
     }
 
     small = TLS13_TICKET_NONCE_STATIC_SZ;
-    medium = small + 20 <= 255 ? small + 20 : 255;
-    big = medium + 20 <= 255 ? small + 20 : 255;
+#if TLS13_TICKET_NONCE_STATIC_SZ + 20 <= 255
+    medium = small + 20;
+#else
+    medium = 255;
+#endif
+#if TLS13_TICKET_NONCE_STATIC_SZ + 20 + 20 <= 255
+    big = small + 20;
+#else
+    big = 255;
+#endif
 
     ExpectIntEQ(test_ticket_nonce_malloc_do(ssl_s, ssl_c, small), TEST_SUCCESS);
     ExpectPtrEq(ssl_c->session->ticketNonce.data,

diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c
@@ -2285,7 +2285,7 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
         /* Bottom 7 bits are the number of bytes to calculate length with.
          * Note: 0 indicates indefinite length encoding *not* 0 bytes of length.
          */
-        word32 bytes = (word32)b & 0x7FU;
+        int bytes = (int)(b & 0x7F);
         int minLen;
 
         /* Calculate minimum length to be encoded with bytes. */
@@ -2297,10 +2297,11 @@ int GetLength_ex(const byte* input, word32* inOutIdx, int* len, word32 maxIdx,
             minLen = 0x80;
         }
         /* Only support up to the number of bytes that fit into return var. */
-        else if (bytes > sizeof(length)) {
+        else if (bytes > (int)sizeof(length)) {
             WOLFSSL_MSG("GetLength - overlong data length spec");
             return ASN_PARSE_E;
-        } else {
+        }
+        else {
             minLen = 1 << ((bytes - 1) * 8);
         }
 

diff --git a/wolfssl/wolfcrypt/sp_int.h b/wolfssl/wolfcrypt/sp_int.h
@@ -692,14 +692,14 @@ typedef struct sp_ecc_ctx {
  *
  * @param  [in]  a  SP integer to update.
  */
-#define sp_clamp(a)                                               \
-    do {                                                          \
-        int ii;                                                   \
-        if ((a)->used > 0) {                                      \
+#define sp_clamp(a)                                                            \
+    do {                                                                       \
+        int 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.