Fix coverity issues on branch 8.4

464364, 464373, 467555 Overflowed constant 456198, 456192, 456200, 456219, 456228, 456230, 456202, 456177, 456168, 456166, 309792, 297704 Logically dead code 456146 Structurally dead code 456221, 456169 Bad bit shift operation 13899, 297759, 456217 Dereference after null check 456213 Array compared against 0 456209 Double unlock 456179, 456201, 456205, 456208, 456151 Unchecked return value 456194 Explicit null dereferenced 456207 Unintentional integer overflow 456132 Data race condition 456116 Argument cannot be negative
Tongsuo-Project · Dec 24, 2024 · 3020dbf · 3020dbf
1 parent 5b9d05a
commit 3020dbf
Show file tree

Hide file tree

Showing 22 changed files with 101 additions and 85 deletions.
diff --git a/apps/delecred.c b/apps/delecred.c
@@ -105,7 +105,8 @@ int delecred_main(int argc, char **argv)
                 ee_key_file = opt_arg();
                 break;
             case OPT_SEC:
-                opt_int(opt_arg(), &valid_time);
+                if (!opt_int(opt_arg(), &valid_time))
+                    goto opthelp;
                 break;
             case OPT_EXPECT_VERIFY_MD:
                 expect_verify_hash = opt_arg();

diff --git a/apps/ec_elgamal.c b/apps/ec_elgamal.c
@@ -330,7 +330,6 @@ int ec_elgamal_main(int argc, char **argv)
     prog = opt_init(argc, argv, ec_elgamal_options);
     if ((o = opt_next()) != OPT_EOF) {
         switch (o) {
-        case OPT_EOF:
         case OPT_ERR:
 opthelp1:
             BIO_printf(bio_err, "%s: Use -help for summary.\n", prog);
@@ -360,11 +359,12 @@ int ec_elgamal_main(int argc, char **argv)
     }
 
     action_sum = encrypt + decrypt + add + sub + mul;
-    if (action_sum == 0) {
-        BIO_printf(bio_err, "No action parameter specified.\n");
-        goto opthelp1;
-    } else if (action_sum != 1) {
-        BIO_printf(bio_err, "Only one action parameter must be specified.\n");
+    if (action_sum != 1) {
+        if (action_sum == 0) {
+            BIO_printf(bio_err, "No action parameter specified.\n");
+        } else {
+            BIO_printf(bio_err, "Only one action parameter must be specified.\n");
+        }
         goto opthelp1;
     }
 

diff --git a/apps/lib/opt.c b/apps/lib/opt.c
@@ -586,7 +586,7 @@ int opt_uintmax(const char *value, ossl_uintmax_t *result)
         opt_number_error(value);
         return 0;
     }
-    *result = (ossl_intmax_t)m;
+    *result = (ossl_uintmax_t)m;
     errno = oerrno;
     return 1;
 }

diff --git a/apps/lib/s_cb.c b/apps/lib/s_cb.c
@@ -76,22 +76,28 @@ int verify_callback(int ok, X509_STORE_CTX *ctx)
     }
     switch (err) {
     case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:
-        BIO_puts(bio_err, "issuer= ");
-        X509_NAME_print_ex(bio_err, X509_get_issuer_name(err_cert),
-                           0, get_nameopt());
-        BIO_puts(bio_err, "\n");
+        if (err_cert != NULL) {
+            BIO_puts(bio_err, "issuer= ");
+            X509_NAME_print_ex(bio_err, X509_get_issuer_name(err_cert),
+                               0, get_nameopt());
+            BIO_puts(bio_err, "\n");
+        }
         break;
     case X509_V_ERR_CERT_NOT_YET_VALID:
     case X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD:
-        BIO_printf(bio_err, "notBefore=");
-        ASN1_TIME_print(bio_err, X509_get0_notBefore(err_cert));
-        BIO_printf(bio_err, "\n");
+        if (err_cert != NULL) {
+            BIO_printf(bio_err, "notBefore=");
+            ASN1_TIME_print(bio_err, X509_get0_notBefore(err_cert));
+            BIO_printf(bio_err, "\n");
+        }
         break;
     case X509_V_ERR_CERT_HAS_EXPIRED:
     case X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD:
-        BIO_printf(bio_err, "notAfter=");
-        ASN1_TIME_print(bio_err, X509_get0_notAfter(err_cert));
-        BIO_printf(bio_err, "\n");
+        if (err_cert != NULL) {
+            BIO_printf(bio_err, "notAfter=");
+            ASN1_TIME_print(bio_err, X509_get0_notAfter(err_cert));
+            BIO_printf(bio_err, "\n");
+        }
         break;
     case X509_V_ERR_NO_EXPLICIT_POLICY:
         if (!verify_args.quiet)

diff --git a/apps/paillier.c b/apps/paillier.c
@@ -447,7 +447,6 @@ int paillier_main(int argc, char **argv)
     prog = opt_init(argc, argv, paillier_options);
     if ((o = opt_next()) != OPT_EOF) {
         switch (o) {
-        case OPT_EOF:
         case OPT_ERR:
 opthelp1:
             BIO_printf(bio_err, "%s: Use -help for summary.\n", prog);
@@ -492,11 +491,12 @@ int paillier_main(int argc, char **argv)
     }
 
     action_sum = keygen + pubgen + key + pub + encrypt + decrypt + add + add_plain + sub + mul;
-    if (action_sum == 0) {
-        BIO_printf(bio_err, "No action parameter specified.\n");
-        goto opthelp1;
-    } else if (action_sum != 1) {
-        BIO_printf(bio_err, "Only one action parameter must be specified.\n");
+    if (action_sum != 1) {
+        if (action_sum == 0) {
+            BIO_printf(bio_err, "No action parameter specified.\n");
+        } else {
+            BIO_printf(bio_err, "Only one action parameter must be specified.\n");
+        }
         goto opthelp1;
     }
 

diff --git a/apps/rehash.c b/apps/rehash.c
@@ -168,7 +168,8 @@ static int add_entry(enum Type type, unsigned int hash, const char *filename,
     if (need_symlink && !ep->need_symlink) {
         ep->need_symlink = 1;
         bp->num_needed++;
-        memcpy(ep->digest, digest, evpmdsize);
+        if (digest != NULL) {
+            memcpy(ep->digest, digest, evpmdsize);
     }
     return 0;
 }
@@ -488,7 +489,7 @@ int rehash_main(int argc, char **argv)
     prog = opt_init(argc, argv, rehash_options);
     while ((o = opt_next()) != OPT_EOF) {
         switch (o) {
-        case OPT_EOF:
+        default:
         case OPT_ERR:
             BIO_printf(bio_err, "%s: Use -help for summary.\n", prog);
             goto end;

diff --git a/apps/speed.c b/apps/speed.c
@@ -608,7 +608,7 @@ static OPT_PAIR bulletproofs_choices[] = {
 # endif
 };
 
-static int bulletproofs_bits[] = {16, 32, 64};
+static int bulletproofs_bits[] = {16, 32, 63};
 static int bulletproofs_agg_max[] = {1, 16, 32};
 
 # define BULLETPROOFS_NUM                   OSSL_NELEM(bulletproofs_choices)
@@ -933,22 +933,25 @@ static int EVP_Update_loop(void *args)
             rc = EVP_DecryptUpdate(ctx, buf, &outl, buf, lengths[testnum]);
             if (rc != 1) {
                 /* reset iv in case of counter overflow */
-                EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1);
+                rc = EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1);
             }
         }
     } else {
         for (count = 0; COND(c[D_EVP][testnum]); count++) {
             rc = EVP_EncryptUpdate(ctx, buf, &outl, buf, lengths[testnum]);
             if (rc != 1) {
                 /* reset iv in case of counter overflow */
-                EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1);
+                rc = EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1);
             }
         }
     }
     if (decrypt)
-        EVP_DecryptFinal_ex(ctx, buf, &outl);
+        rc = EVP_DecryptFinal_ex(ctx, buf, &outl);
     else
-        EVP_EncryptFinal_ex(ctx, buf, &outl);
+        rc = EVP_EncryptFinal_ex(ctx, buf, &outl);
+
+    if (rc == 0)
+        BIO_printf(bio_err, "Error finalizing cipher loop\n");
     return count;
 }
 
@@ -4170,7 +4173,7 @@ int speed_main(int argc, char **argv)
 
 #ifndef OPENSSL_NO_BULLETPROOFS
     for (i = 1; i < sizeof(bp_secrets)/sizeof(bp_secrets[0]); i++) {
-        bp_secrets[i] = (1U << i) - 1;
+        bp_secrets[i] = (1ULL << i) - 1;
     }
 
     if (!(v = BN_new()))
@@ -4184,7 +4187,7 @@ int speed_main(int argc, char **argv)
             continue;           /* Ignore Curve */
 
         for (m = 0; m < BULLETPROOFS_BITS_NUM; m++) {
-            bp_secrets[0] = (1U << bulletproofs_bits[m]) - 1;
+            bp_secrets[0] = (1ULL << bulletproofs_bits[m]) - 1;
 
             for (n = 0; n < BULLETPROOFS_AGG_MAX_NUM; n++) {
                 bp_pp[testnum][m][n] = BP_PUB_PARAM_new_by_curve_id(test_bulletproofs_curves[testnum].nid,
@@ -4224,7 +4227,7 @@ int speed_main(int argc, char **argv)
                     }
 
                     bp_ctx[testnum][m][n][j] = BP_RANGE_CTX_new(bp_pp[testnum][m][n], bp_witness[testnum][m][n][j], bp_transcript[testnum][m][n]);
-                    if (bp_ctx[testnum][m][n] == NULL)
+                    if (bp_ctx[testnum][m][n][j] == NULL)
                         goto end;
 
                     if (!BP_RANGE_PROOF_prove(bp_ctx[testnum][m][n][j], bp_proof[testnum][m][n])) {

diff --git a/crypto/ec/ec_elgamal_crypt.c b/crypto/ec/ec_elgamal_crypt.c
@@ -92,18 +92,19 @@ EC_ELGAMAL_CTX *EC_ELGAMAL_CTX_new(EC_KEY *key, const EC_POINT *h, int32_t flag)
     }
 #endif
 
-    EC_KEY_up_ref(key);
+    if (!EC_KEY_up_ref(key))
+        goto err;
     ctx->key = key;
     ctx->flag = flag;
 
     return ctx;
-#ifndef OPENSSL_NO_TWISTED_EC_ELGAMAL
 err:
+#ifndef OPENSSL_NO_TWISTED_EC_ELGAMAL
     OPENSSL_free(buf);
     BN_CTX_free(bn_ctx);
+#endif
     EC_ELGAMAL_CTX_free(ctx);
     return NULL;
-#endif
 }
 
 EC_ELGAMAL_CTX *EC_ELGAMAL_CTX_dup(EC_ELGAMAL_CTX *ctx)

diff --git a/crypto/encode_decode/encoder_lib.c b/crypto/encode_decode/encoder_lib.c
@@ -543,7 +543,7 @@ static int encoder_process(struct encoder_process_data_st *data)
         /* Preparations */
 
         switch (ok) {
-        case 0:
+        default:
             break;
         case -1:
             /*

diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c
@@ -223,11 +223,17 @@ int CRYPTO_secure_allocated(const void *ptr)
 
 size_t CRYPTO_secure_used(void)
 {
+    size_t ret = 0;
+
 #ifndef OPENSSL_NO_SECURE_MEMORY
-    return secure_mem_used;
-#else
-    return 0;
+    if (!CRYPTO_THREAD_read_lock(sec_malloc_lock))
+        return 0;
+
+    ret = secure_mem_used;
+
+    CRYPTO_THREAD_unlock(sec_malloc_lock);
 #endif /* OPENSSL_NO_SECURE_MEMORY */
+    return ret;
 }
 
 size_t CRYPTO_secure_actual_size(void *ptr)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
@@ -637,7 +637,7 @@ int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
         if (!ossl_provider_up_ref(actualtmp)) {
             ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
             actualtmp = NULL;
-            goto err;
+            return 0;
         }
         *actualprov = actualtmp;
     }
@@ -661,8 +661,6 @@ int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
 
  err:
     CRYPTO_THREAD_unlock(store->lock);
-    if (actualprov != NULL)
-        ossl_provider_free(*actualprov);
     return 0;
 }
 

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
@@ -284,11 +284,13 @@ int X509_STORE_copy(X509_STORE *dest, const X509_STORE *src)
         for (i = 0; i < num; i++) {
             obj = sk_X509_OBJECT_value(src->objs, i);
             if (obj->type == X509_LU_X509) {
-                X509_STORE_add_cert(dest, obj->data.x509);
+                if (!X509_STORE_add_cert(dest, obj->data.x509))
+                    return 0;
             } else if (obj->type == X509_LU_CRL) {
-                X509_STORE_add_crl(dest, obj->data.crl);
+                if (!X509_STORE_add_crl(dest, obj->data.crl))
+                    return 0;
             } else {
-                /* abort(); */
+                return 0;
             }
         }
     }

diff --git a/crypto/zkp/bulletproofs/bulletproofs_encode.c b/crypto/zkp/bulletproofs/bulletproofs_encode.c
@@ -842,7 +842,7 @@ BP_RANGE_PROOF *BP_RANGE_PROOF_decode(const unsigned char *in, size_t size)
     proof->T2 = sk_EC_POINT_value(sk_point, 3);
 
     sk_bn = zkp_stack_of_bignum_decode(p, &len, bn_len);
-    if (sk_point == NULL)
+    if (sk_bn == NULL)
         goto err;
     p += len;
 
@@ -1114,7 +1114,7 @@ BP_R1CS_PROOF *BP_R1CS_PROOF_decode(const unsigned char *in, size_t size)
 #endif
 
     sk_bn = zkp_stack_of_bignum_decode(p, &len, bn_len);
-    if (sk_point == NULL)
+    if (sk_bn == NULL)
         goto err;
     p += len;
 

diff --git a/crypto/zkp/bulletproofs/inner_product.c b/crypto/zkp/bulletproofs/inner_product.c
@@ -172,6 +172,7 @@ bp_inner_product_proof_t *bp_inner_product_proof_new(bp_inner_product_ctx_t *ctx
 
     if (ctx == NULL || ctx->pp == NULL) {
         ERR_raise(ERR_LIB_ZKP_BP, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
     }
 
     n = sk_EC_POINT_num(ctx->pp->sk_G);

diff --git a/crypto/zkp/bulletproofs/r1cs.c b/crypto/zkp/bulletproofs/r1cs.c
@@ -836,7 +836,7 @@ int BP_R1CS_PROOF_verify(BP_R1CS_CTX *ctx, BP_R1CS_PROOF *proof)
 
     v_n = sk_BP_VARIABLE_num(witness->sk_V);
     lg_n = sk_EC_POINT_num(ip_proof->sk_L);
-    if (padded_n != 1 << lg_n) {
+    if (lg_n < 0 || (lg_n >= sizeof(int) * 8) || (padded_n != 1U << lg_n)) {
         ERR_raise(ERR_LIB_ZKP_BP, ERR_R_PASSED_INVALID_ARGUMENT);
         goto err;
     }

diff --git a/crypto/zkp/bulletproofs/r1cs_linear_combination.c b/crypto/zkp/bulletproofs/r1cs_linear_combination.c
@@ -210,7 +210,8 @@ BP_R1CS_LINEAR_COMBINATION *BP_R1CS_LINEAR_COMBINATION_dup(const BP_R1CS_LINEAR_
         if (item_dup == NULL)
             goto err;
 
-        sk_BP_R1CS_LINEAR_COMBINATION_ITEM_push(ret->items, item_dup);
+        if (sk_BP_R1CS_LINEAR_COMBINATION_ITEM_push(ret->items, item_dup) <= 0)
+            goto err;
     }
 
     ret->type = lc->type;
@@ -427,12 +428,12 @@ int BP_R1CS_LINEAR_COMBINATION_raw_mul(BP_R1CS_LINEAR_COMBINATION **output,
     BN_CTX_free(bn_ctx);
     return 1;
 err:
-    if (output == NULL)
-        output = NULL;
-    if (left == NULL)
-        left = NULL;
-    if (right == NULL)
-        right = NULL;
+    if (output != NULL)
+        *output = NULL;
+    if (left != NULL)
+        *left = NULL;
+    if (right != NULL)
+        *right = NULL;
 
     BP_R1CS_LINEAR_COMBINATION_free(llc);
     BP_R1CS_LINEAR_COMBINATION_free(rlc);

diff --git a/crypto/zkp/nizk/nizk_encode.c b/crypto/zkp/nizk/nizk_encode.c
@@ -493,7 +493,7 @@ NIZK_PLAINTEXT_KNOWLEDGE_PROOF *NIZK_PLAINTEXT_KNOWLEDGE_PROOF_decode(const unsi
     proof->B = sk_EC_POINT_value(sk_point, 1);
 
     sk_bn = zkp_stack_of_bignum_decode(p, &len, bn_len);
-    if (sk_point == NULL)
+    if (sk_bn == NULL)
         goto err;
     p += len;
 
@@ -892,7 +892,7 @@ NIZK_DLOG_KNOWLEDGE_PROOF *NIZK_DLOG_KNOWLEDGE_PROOF_decode(const unsigned char
     proof->A = sk_EC_POINT_value(sk_point, 0);
 
     sk_bn = zkp_stack_of_bignum_decode(p, &len, bn_len);
-    if (sk_point == NULL)
+    if (sk_bn == NULL)
         goto err;
     p += len;
 
@@ -1092,7 +1092,7 @@ NIZK_DLOG_EQUALITY_PROOF *NIZK_DLOG_EQUALITY_PROOF_decode(const unsigned char *i
     proof->A2 = sk_EC_POINT_value(sk_point, 1);
 
     sk_bn = zkp_stack_of_bignum_decode(p, &len, bn_len);
-    if (sk_point == NULL)
+    if (sk_bn == NULL)
         goto err;
     p += len;