numerous fixes for memory errors reported by clang-tidy, most of them…

… true positives, unmasked by CPPFLAGS=-DNO_WOLFSSL_MEMORY: clang-analyzer-unix.Malloc, clang-analyzer-core.NullDereference, clang-analyzer-core.uninitialized.Assign, clang-analyzer-core.UndefinedBinaryOperatorResult, and clang-analyzer-unix.API (re malloc(0)).

several fixes for memory error reported by cppcheck:

wolfcrypt/src/ecc.c: fix for cppcheck oppositeInnerCondition from cppcheck-2.16.0 in _ecc_make_key_ex(), and fixes for related unhandled errors discovered by manual inspection;

wolfcrypt/test/test.c: fix XREALLOC call in memcb_test() to resolve cppcheck-detected memleak.

src/crl.c

@@ -1027,6 +1027,7 @@ static int DupX509_CRL(WOLFSSL_X509_CRL *dupl, const WOLFSSL_X509_CRL* crl)
             if (dupl->monitors[0].path != NULL) {
                 XFREE(dupl->monitors[0].path, dupl->heap,
                         DYNAMIC_TYPE_CRL_MONITOR);
+                dupl->monitors[0].path = NULL;
             }
             return MEMORY_E;
         }

src/ssl.c

@@ -7991,7 +7991,7 @@ int wolfSSL_i2d_PKCS8_PKEY(WOLFSSL_PKCS8_PRIV_KEY_INFO* key, unsigned char** pp)
         return WOLFSSL_FATAL_ERROR;
     len = (int)keySz;
 
-    if (pp == NULL)
+    if ((pp == NULL) || (len == 0))
         return len;
 
     if (*pp == NULL) {

src/tls13.c

@@ -6960,7 +6960,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
         echX = TLSX_Find(ssl->extensions, TLSX_ECH);
 
         if (echX == NULL)
-            return WOLFSSL_FATAL_ERROR;
+            ERROR_OUT(WOLFSSL_FATAL_ERROR, exit_dch);
 
         ((WOLFSSL_ECH*)echX->data)->aad = input + HANDSHAKE_HEADER_SZ;
         ((WOLFSSL_ECH*)echX->data)->aadLen = helloSz;

src/x509.c

@@ -1690,7 +1690,6 @@ WOLFSSL_v3_ext_method* wolfSSL_X509V3_EXT_get(WOLFSSL_X509_EXTENSION* ex)
         WOLFSSL_MSG("Failed to get nid from passed extension object");
         return NULL;
     }
-    XMEMSET(&method, 0, sizeof(WOLFSSL_v3_ext_method));
     switch (nid) {
         case WC_NID_basic_constraints:
             break;

tests/api.c

@@ -33411,7 +33411,7 @@ static int test_wc_dilithium_check_key(void)
         &privCheckKeyLen, pubCheckKey, &pubCheckKeyLen), 0);
 
     /* Modify hash. */
-    if (pubCheckKey != NULL) {
+    if ((pubCheckKey != NULL) && EXPECT_SUCCESS()) {
         pubCheckKey[0] ^= 0x80;
         ExpectIntEQ(wc_dilithium_import_key(NULL, 0, NULL, 0, NULL),
             WC_NO_ERR_TRACE(BAD_FUNC_ARG));
@@ -78037,7 +78037,6 @@ static int test_wolfSSL_d2i_OCSP_CERTID(void)
 {
     EXPECT_DECLS;
 #if (defined(OPENSSL_ALL) || defined(WOLFSSL_HAPROXY)) && defined(HAVE_OCSP)
-    WOLFSSL_OCSP_CERTID* certId;
     WOLFSSL_OCSP_CERTID* certIdGood;
     WOLFSSL_OCSP_CERTID* certIdBad;
     const unsigned char* rawCertIdPtr;
@@ -78056,40 +78055,49 @@ static int test_wolfSSL_d2i_OCSP_CERTID(void)
 
     /* If the cert ID is NULL the function should allocate it and copy the
      * data to it. */
-    certId = NULL;
-    ExpectNotNull(certId = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
-        sizeof(rawCertId)));
-    ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
-    if (certId != NULL) {
-        XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
-        XFREE(certId, NULL, DYNAMIC_TYPE_OPENSSL);
+    {
+        WOLFSSL_OCSP_CERTID* certId = NULL;
+        ExpectNotNull(certId = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
+                                                       sizeof(rawCertId)));
+        ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
+        if (certId != NULL) {
+            XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
+            XFREE(certId, NULL, DYNAMIC_TYPE_OPENSSL);
+        }
     }
 
     /* If the cert ID is not NULL the function will just copy the data to it. */
-    ExpectNotNull(certId = (WOLFSSL_OCSP_CERTID*)XMALLOC(sizeof(*certId), NULL,
-        DYNAMIC_TYPE_TMP_BUFFER));
-    ExpectNotNull(certId);
-    ExpectNotNull(XMEMSET(certId, 0, sizeof(*certId)));
-
-    /* Reset rawCertIdPtr since it was push forward in the previous call. */
-    rawCertIdPtr = &rawCertId[0];
-    ExpectNotNull(certIdGood = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
-        sizeof(rawCertId)));
-    ExpectPtrEq(certIdGood, certId);
-    ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
-    if (certId != NULL) {
-        XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
-        XFREE(certId, NULL, DYNAMIC_TYPE_TMP_BUFFER);
-        certId = NULL;
+    {
+        WOLFSSL_OCSP_CERTID* certId = NULL;
+        ExpectNotNull(certId = (WOLFSSL_OCSP_CERTID*)XMALLOC(sizeof(*certId), NULL,
+                                                             DYNAMIC_TYPE_TMP_BUFFER));
+        ExpectNotNull(certId);
+        if (certId != NULL)
+            XMEMSET(certId, 0, sizeof(*certId));
+
+        /* Reset rawCertIdPtr since it was push forward in the previous call. */
+        rawCertIdPtr = &rawCertId[0];
+        ExpectNotNull(certIdGood = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
+                                                           sizeof(rawCertId)));
+        ExpectPtrEq(certIdGood, certId);
+        ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
+        if (certId != NULL) {
+            XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
+            XFREE(certId, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+            certId = NULL;
+        }
     }
 
     /* The below tests should fail when passed bad parameters. NULL should
      * always be returned. */
-    ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(NULL, &rawCertIdPtr,
-        sizeof(rawCertId)));
-    ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, NULL,
-        sizeof(rawCertId)));
-    ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr, 0));
+    {
+        WOLFSSL_OCSP_CERTID* certId = NULL;
+        ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(NULL, &rawCertIdPtr,
+                                                       sizeof(rawCertId)));
+        ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, NULL,
+                                                       sizeof(rawCertId)));
+        ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr, 0));
+    }
 #endif
     return EXPECT_RESULT();
 }
@@ -84988,6 +84996,7 @@ static int test_wolfSSL_PEM_X509_INFO_read_bio(void)
 
     ExpectIntEQ(0, XSTRNCMP(subject, exp1, sizeof(exp1)));
     XFREE(subject, 0, DYNAMIC_TYPE_OPENSSL);
+    subject = NULL;
     X509_INFO_free(info);
     info = NULL;
 
@@ -84997,6 +85006,7 @@ static int test_wolfSSL_PEM_X509_INFO_read_bio(void)
 
     ExpectIntEQ(0, XSTRNCMP(subject, exp2, sizeof(exp2)));
     XFREE(subject, 0, DYNAMIC_TYPE_OPENSSL);
+    subject = NULL;
     X509_INFO_free(info);
     ExpectNull(info = sk_X509_INFO_pop(sk));
 

wolfcrypt/src/asn.c

@@ -28804,6 +28804,13 @@ int SetNameEx(byte* output, word32 outputSz, CertName* name, void* heap)
         ret = 0;
     }
 
+    if (items == 0) {
+        /* if zero items, short-circuit return to avoid frivolous zero-size
+         * allocations.
+         */
+        return 0;
+    }
+
     /* Allocate dynamic data items. */
     dataASN = (ASNSetData*)XMALLOC(items * sizeof(ASNSetData), heap,
                                    DYNAMIC_TYPE_TMP_BUFFER);
@@ -34171,23 +34178,26 @@ static int EccSpecifiedECDomainDecode(const byte* input, word32 inSz,
     }
     #endif /* WOLFSSL_ECC_CURVE_STATIC */
 
+    if ((ret == 0) && (curveSz)) {
+        *curveSz = curve->size;
+    }
+
     if (key) {
         /* Store parameter set in key. */
-        if ((ret == 0) && (wc_ecc_set_custom_curve(key, curve) < 0)) {
-            ret = ASN_PARSE_E;
-        }
         if (ret == 0) {
-            /* The parameter set was allocated.. */
-            key->deallocSet = 1;
+            if (wc_ecc_set_custom_curve(key, curve) < 0) {
+                ret = ASN_PARSE_E;
+            }
+            else {
+                /* The parameter set was allocated.. */
+                key->deallocSet = 1;
+                /* Don't deallocate below. */
+                curve = NULL;
+            }
         }
     }
 
-    if ((ret == 0) && (curveSz)) {
-        *curveSz = curve->size;
-    }
-
-    if ((ret != 0) && (curve != NULL)) {
-        /* Failed to set parameters so free parameter set. */
+    if (curve != NULL) { /* NOLINT(clang-analyzer-unix.Malloc) */
         wc_ecc_free_curve(curve, heap);
     }
 

wolfcrypt/src/dh.c

@@ -2036,19 +2036,21 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 #ifndef WOLFSSL_SP_NO_2048
     if (mp_count_bits(&key->p) == 2048) {
         if (mp_init(y) != MP_OKAY)
-            return MP_INIT_E;
+            ret = MP_INIT_E;
 
-        SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
+        if (ret == 0) {
+            SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
 
-        if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
-            ret = MP_READ_E;
+            if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
+                ret = MP_READ_E;
 
-        if (ret == 0)
-            ret = sp_DhExp_2048(y, priv, privSz, &key->p, agree, agreeSz);
+            if (ret == 0)
+                ret = sp_DhExp_2048(y, priv, privSz, &key->p, agree, agreeSz);
 
-        mp_clear(y);
+            mp_clear(y);
 
-        RESTORE_VECTOR_REGISTERS();
+            RESTORE_VECTOR_REGISTERS();
+        }
 
         /* make sure agree is > 1 (SP800-56A, 5.7.1.1) */
         if ((ret == 0) &&
@@ -2070,19 +2072,21 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 #ifndef WOLFSSL_SP_NO_3072
     if (mp_count_bits(&key->p) == 3072) {
         if (mp_init(y) != MP_OKAY)
-            return MP_INIT_E;
+            ret = MP_INIT_E;
 
-        SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
+        if (ret == 0) {
+            SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
 
-        if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
-            ret = MP_READ_E;
+            if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
+                ret = MP_READ_E;
 
-        if (ret == 0)
-            ret = sp_DhExp_3072(y, priv, privSz, &key->p, agree, agreeSz);
+            if (ret == 0)
+                ret = sp_DhExp_3072(y, priv, privSz, &key->p, agree, agreeSz);
 
-        mp_clear(y);
+            mp_clear(y);
 
-        RESTORE_VECTOR_REGISTERS();
+            RESTORE_VECTOR_REGISTERS();
+        }
 
         /* make sure agree is > 1 (SP800-56A, 5.7.1.1) */
         if ((ret == 0) &&
@@ -2104,19 +2108,21 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 #ifdef WOLFSSL_SP_4096
     if (mp_count_bits(&key->p) == 4096) {
         if (mp_init(y) != MP_OKAY)
-            return MP_INIT_E;
+            ret = MP_INIT_E;
 
-        SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
+        if (ret == 0) {
+            SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
 
-        if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
-            ret = MP_READ_E;
+            if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
+                ret = MP_READ_E;
 
-        if (ret == 0)
-            ret = sp_DhExp_4096(y, priv, privSz, &key->p, agree, agreeSz);
+            if (ret == 0)
+                ret = sp_DhExp_4096(y, priv, privSz, &key->p, agree, agreeSz);
 
-        mp_clear(y);
+            mp_clear(y);
 
-        RESTORE_VECTOR_REGISTERS();
+            RESTORE_VECTOR_REGISTERS();
+        }
 
         /* make sure agree is > 1 (SP800-56A, 5.7.1.1) */
         if ((ret == 0) &&

wolfcrypt/src/dilithium.c

@@ -2195,7 +2195,7 @@ static int dilithium_rej_ntt_poly_ex(wc_Shake* shake128, byte* seed, sword32* a,
 static int dilithium_rej_ntt_poly(wc_Shake* shake128, byte* seed, sword32* a,
     void* heap)
 {
-    int ret;
+    int ret = 0;
 #if defined(WOLFSSL_SMALL_STACK)
     byte* h = NULL;
 #else
@@ -2212,7 +2212,8 @@ static int dilithium_rej_ntt_poly(wc_Shake* shake128, byte* seed, sword32* a,
     }
 #endif
 
-    ret = dilithium_rej_ntt_poly_ex(shake128, seed, a, h);
+    if (ret == 0)
+        ret = dilithium_rej_ntt_poly_ex(shake128, seed, a, h);
 
 #if defined(WOLFSSL_SMALL_STACK)
     XFREE(h, heap, DYNAMIC_TYPE_DILITHIUM);
@@ -6076,6 +6077,7 @@ static int dilithium_sign_with_seed_mu(dilithium_key* key,
             ret = MEMORY_E;
         }
         else {
+            XMEMSET(key->s1, 0, params->aSz);
             key->s2 = key->s1  + params->s1Sz / sizeof(*s1);
             key->t0 = key->s2  + params->s2Sz / sizeof(*s2);
         }
@@ -7223,6 +7225,9 @@ static int dilithium_verify_mu(dilithium_key* key, const byte* mu,
         if (key->a == NULL) {
             ret = MEMORY_E;
         }
+        else {
+            XMEMSET(key->a, 0, params->aSz);
+        }
     }
 #endif
     if (ret == 0) {
@@ -7237,6 +7242,9 @@ static int dilithium_verify_mu(dilithium_key* key, const byte* mu,
         if (key->t1 == NULL) {
             ret = MEMORY_E;
         }
+        else {
+            XMEMSET(key->t1, 0, params->s2Sz);
+        }
     }
 #endif
     if (ret == 0) {
@@ -7259,6 +7267,7 @@ static int dilithium_verify_mu(dilithium_key* key, const byte* mu,
             ret = MEMORY_E;
         }
         else {
+            XMEMSET(z, 0, allocSz);
             c   = z  + params->s1Sz / sizeof(*z);
             w   = c  + DILITHIUM_N;
 #ifndef WC_DILITHIUM_CACHE_PUB_VECTORS
@@ -7387,6 +7396,7 @@ static int dilithium_verify_mu(dilithium_key* key, const byte* mu,
             ret = MEMORY_E;
         }
         else {
+            XMEMSET(z, 0, allocSz);
             c     = z + params->s1Sz / sizeof(*t1);
             w     = c + DILITHIUM_N;
             t1    = w + DILITHIUM_N;
@@ -8908,6 +8918,7 @@ int wc_dilithium_check_key(dilithium_key* key)
             ret = MEMORY_E;
         }
         else {
+            XMEMSET(s1, 0, allocSz);
             s2 = s1 + params->s1Sz / sizeof(*s1);
             t0 = s2 + params->s2Sz / sizeof(*s2);
             t  = t0 + params->s2Sz / sizeof(*t0);
@@ -9197,6 +9208,9 @@ int wc_dilithium_import_public(const byte* in, word32 inLen, dilithium_key* key)
             if (key->t1 == NULL) {
                 ret = MEMORY_E;
             }
+            else {
+                XMEMSET(key->t1, 0, key->params->s2Sz);
+            }
         }
     #endif
     }
@@ -9213,6 +9227,9 @@ int wc_dilithium_import_public(const byte* in, word32 inLen, dilithium_key* key)
             if (key->a == NULL) {
                 ret = MEMORY_E;
             }
+            else {
+                XMEMSET(key->a, 0, params->aSz);
+            }
         }
     #endif
     }
@@ -9282,6 +9299,9 @@ static int dilithium_set_priv_key(const byte* priv, word32 privSz,
             if (key->a == NULL) {
                 ret = MEMORY_E;
             }
+            else {
+                XMEMSET(key->a, 0, params->aSz);
+            }
         }
     }
 #endif
@@ -9303,6 +9323,9 @@ static int dilithium_set_priv_key(const byte* priv, word32 privSz,
         if (key->s1 == NULL) {
             ret = MEMORY_E;
         }
+        else {
+            XMEMSET(key->s1, 0, params->s1Sz + params->s2Sz + params->s2Sz);
+        }
         if (ret == 0) {
             /* Set pointers into allocated memory. */
             key->s2 = key->s1 + params->s1Sz / sizeof(*key->s1);