Fix coverity issues on 8.3 branch

470740, 470741, 470742 Explicit null dereferenced 464373 Overflowed constant 265647 Explicit null dereferenced 464516, 456146 Structurally dead code 297397, 297533, 297551, 456144, 456113, 297761, 297738, 297685, 297667, 297378, 297643, 297577, 456151 Unchecked return value 456130, 456136 Unchecked return value from library 456133 Explicit null dereferenced 456132 Data race condition 456116 Argument cannot be negative 13896, 297759, 297445 Dereference after null check 297704 Logically dead code 13891 Division or modulo by zero
Tongsuo-Project · Dec 22, 2024 · 3a8fdca · 3a8fdca
1 parent ddd2b94
commit 3a8fdca
Show file tree

Hide file tree

Showing 22 changed files with 147 additions and 98 deletions.
diff --git a/apps/delecred.c b/apps/delecred.c
@@ -96,7 +96,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/ocsp.c b/apps/ocsp.c
@@ -456,12 +456,14 @@ int ocsp_main(int argc, char **argv)
             rca_filename = opt_arg();
             break;
         case OPT_NMIN:
-            opt_int(opt_arg(), &nmin);
+            if (!opt_int(opt_arg(), &nmin))
+                goto opthelp;
             if (ndays == -1)
                 ndays = 0;
             break;
         case OPT_REQUEST:
-            opt_int(opt_arg(), &accept_count);
+            if (!opt_int(opt_arg(), &accept_count))
+                goto opthelp;
             break;
         case OPT_NDAYS:
             ndays = atoi(opt_arg());

diff --git a/apps/rehash.c b/apps/rehash.c
@@ -181,7 +181,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;
 }
@@ -482,7 +483,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/s_cb.c b/apps/s_cb.c
@@ -74,22 +74,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_puts(bio_err, "notBefore=");
+            ASN1_TIME_print(bio_err, X509_get0_notBefore(err_cert));
+            BIO_puts(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_puts(bio_err, "notAfter=");
+            ASN1_TIME_print(bio_err, X509_get0_notAfter(err_cert));
+            BIO_puts(bio_err, "\n");
+        }
         break;
     case X509_V_ERR_NO_EXPLICIT_POLICY:
         if (!verify_args.quiet)

diff --git a/apps/s_client.c b/apps/s_client.c
@@ -3330,7 +3330,6 @@ int s_client_main(int argc, char **argv)
         }
     }
 
-    ret = 0;
  shut:
     if (in_init)
         print_stuff(bio_c_out, con, full_log);

diff --git a/apps/s_time.c b/apps/s_time.c
@@ -337,9 +337,13 @@ int s_time_main(int argc, char **argv)
     printf
         ("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
          nConn, totalTime, ((double)nConn / totalTime), bytes_read);
-    printf
-        ("%d connections in %ld real seconds, %ld bytes read per connection\n",
-         nConn, (long)time(NULL) - finishtime + maxtime, bytes_read / nConn);
+    if (nConn > 0)
+        printf
+            ("%d connections in %ld real seconds, %ld bytes read per connection\n",
+             nConn, (long)time(NULL) - finishtime + maxtime, bytes_read / nConn);
+    else
+        printf("0 connections in %ld real seconds\n",
+               (long)time(NULL) - finishtime + maxtime);
 
     ret = 0;
 

diff --git a/apps/speed.c b/apps/speed.c
@@ -746,9 +746,10 @@ static int HMAC_loop(void *args)
     int count;
 
     for (count = 0; COND(c[D_HMAC][testnum]); count++) {
-        HMAC_Init_ex(hctx, NULL, 0, NULL, NULL);
-        HMAC_Update(hctx, buf, lengths[testnum]);
-        HMAC_Final(hctx, hmac, NULL);
+        if (!HMAC_Init_ex(hctx, NULL, 0, NULL, NULL)
+            || !HMAC_Update(hctx, buf, lengths[testnum])
+            || !HMAC_Final(hctx, hmac, NULL))
+            return -1;
     }
     return count;
 }
@@ -1004,15 +1005,17 @@ 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);
+                if (!EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1))
+                    return -1;
             }
         }
     } else {
         for (count = 0; COND(nb_iter); 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);
+                if (!EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, -1))
+                    return -1;
             }
         }
     }
@@ -1257,7 +1260,8 @@ static int ECDH_EVP_derive_key_loop(void *args)
     size_t *outlen = &(tempargs->outlen[testnum]);
 
     for (count = 0; COND(ecdh_c[testnum][0]); count++)
-        EVP_PKEY_derive(ctx, derived_secret, outlen);
+        if (EVP_PKEY_derive(ctx, derived_secret, outlen) <= 0)
+            return -1;
 
     return count;
 }

diff --git a/crypto/asn1/x_algor.c b/crypto/asn1/x_algor.c
@@ -110,13 +110,18 @@ int X509_ALGOR_copy(X509_ALGOR *dest, const X509_ALGOR *src)
         if ((dest->algorithm = OBJ_dup(src->algorithm)) == NULL)
 	    return 0;
 
-    if (src->parameter)
+    if (src->parameter) {
+        dest->parameter = ASN1_TYPE_new();
+        if (dest->parameter == NULL)
+            return 0;
+
         /* Assuming this is also correct for a BOOL.
          * set does copy as a side effect.
          */
-        if (ASN1_TYPE_set1(dest->parameter, 
-              src->parameter->type, src->parameter->value.ptr) == 0)
-	    return 0;
+        if (ASN1_TYPE_set1(dest->parameter,
+                src->parameter->type, src->parameter->value.ptr) == 0)
+            return 0;
+    }
 
     return 1;
 }
diff --git a/crypto/comp/c_zlib.c b/crypto/comp/c_zlib.c
@@ -384,7 +384,12 @@ static int bio_zlib_read(BIO *b, char *out, int outl)
             COMPerr(COMP_F_BIO_ZLIB_READ, ERR_R_MALLOC_FAILURE);
             return 0;
         }
-        inflateInit(zin);
+        ret = inflateInit(zin);
+        if (ret != Z_OK) {
+            COMPerr(COMP_F_BIO_ZLIB_READ, COMP_R_ZLIB_INFLATE_ERROR);
+            ERR_add_error_data(2, "zlib error:", zError(ret));
+            return 0;
+        }
         zin->next_in = ctx->ibuf;
         zin->avail_in = 0;
     }
@@ -447,7 +452,12 @@ static int bio_zlib_write(BIO *b, const char *in, int inl)
         }
         ctx->optr = ctx->obuf;
         ctx->ocount = 0;
-        deflateInit(zout, ctx->comp_level);
+        ret = deflateInit(zout, ctx->comp_level);
+        if (ret != Z_OK) {
+            COMPerr(COMP_F_BIO_ZLIB_WRITE, COMP_R_ZLIB_DEFLATE_ERROR);
+            ERR_add_error_data(2, "zlib error:", zError(ret));
+            return 0;
+        }
         zout->next_out = ctx->obuf;
         zout->avail_out = ctx->obufsize;
     }

diff --git a/crypto/ec/ec_elgamal.c b/crypto/ec/ec_elgamal.c
@@ -618,9 +618,10 @@ int EC_ELGAMAL_encrypt(EC_ELGAMAL_CTX *ctx, EC_ELGAMAL_CIPHERTEXT *r, int32_t pl
             goto err;
     }
 
-    EC_GROUP_get_order(ctx->key->group, ord, bn_ctx);
-    BN_rand_range(rand, ord);
+    if (!EC_GROUP_get_order(ctx->key->group, ord, bn_ctx))
+        goto err;
 
+    BN_rand_range(rand, ord);
     BN_set_word(bn_plain, plaintext);
 
     if (!EC_POINT_mul(ctx->key->group, r->C1, rand, NULL, NULL, bn_ctx))

diff --git a/crypto/evp/evp_lib.c b/crypto/evp/evp_lib.c
@@ -197,6 +197,8 @@ int EVP_Cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
 
 const EVP_CIPHER *EVP_CIPHER_CTX_cipher(const EVP_CIPHER_CTX *ctx)
 {
+    if (ctx == NULL)
+        return NULL;
     return ctx->cipher;
 }
 
@@ -294,12 +296,12 @@ int EVP_CIPHER_CTX_key_length(const EVP_CIPHER_CTX *ctx)
 
 int EVP_CIPHER_nid(const EVP_CIPHER *cipher)
 {
-    return cipher->nid;
+    return (cipher == NULL) ? NID_undef : cipher->nid;
 }
 
 int EVP_CIPHER_CTX_nid(const EVP_CIPHER_CTX *ctx)
 {
-    return ctx->cipher->nid;
+    return EVP_CIPHER_nid(ctx->cipher);
 }
 
 int EVP_MD_block_size(const EVP_MD *md)

diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c
@@ -208,11 +208,17 @@ int CRYPTO_secure_allocated(const void *ptr)
 
 size_t CRYPTO_secure_used(void)
 {
+    size_t ret = 0;
+
 #ifdef OPENSSL_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_SECURE_MEMORY */
+    return ret;
 }
 
 size_t CRYPTO_secure_actual_size(void *ptr)

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
@@ -265,11 +265,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/ssl/ssl_dc.c b/ssl/ssl_dc.c
@@ -291,19 +291,25 @@ int DC_sign(DELEGATED_CREDENTIAL *dc, EVP_PKEY *dc_pkey,
         goto end;
     }
 
-    dc_pkey_raw_len = i2d_PUBKEY(dc_pkey, NULL);
-    if (dc_pkey_raw_len <= 0) {
+    ret = i2d_PUBKEY(dc_pkey, NULL);
+    if (ret <= 0) {
         SSLerr(SSL_F_DC_SIGN, ERR_R_INTERNAL_ERROR);
         goto end;
     }
 
-    if ((dc_pkey_raw = OPENSSL_malloc(dc_pkey_raw_len)) == NULL) {
+    if ((dc_pkey_raw = OPENSSL_malloc(ret)) == NULL) {
         SSLerr(SSL_F_DC_SIGN, ERR_R_MALLOC_FAILURE);
         goto end;
     }
 
     dc_pkey_raw_index = dc_pkey_raw;
-    dc_pkey_raw_len = i2d_PUBKEY(dc_pkey, &dc_pkey_raw_index);
+    ret = i2d_PUBKEY(dc_pkey, &dc_pkey_raw_index);
+    if (ret <= 0) {
+        SSLerr(SSL_F_DC_SIGN, ERR_R_INTERNAL_ERROR);
+        goto end;
+    }
+
+    dc_pkey_raw_len = ret;
 
     if (!tls1_lookup_sigalg_by_pkey_and_hash(
             dc_pkey, expect_verify_hash,

diff --git a/ssl/statem_ntls/extensions.c b/ssl/statem_ntls/extensions.c
@@ -1014,19 +1014,9 @@ static int init_alpn(SSL *s, unsigned int context)
 static int final_alpn(SSL *s, unsigned int context, int sent)
 {
     if (!s->server && !sent && s->session->ext.alpn_selected != NULL)
-            s->ext.early_data_ok = 0;
-        return 1;
+        s->ext.early_data_ok = 0;
 
-    /*
-     * Call alpn_select callback if needed.  Has to be done after SNI and
-     * cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3
-     * we also have to do this before we decide whether to accept early_data.
-     * In TLSv1.3 we've already negotiated our cipher so we do this call now.
-     * For < TLSv1.3 we defer it until after cipher negotiation.
-     *
-     * On failure SSLfatal_ntls() already called.
-     */
-    return tls_handle_alpn_ntls(s);
+    return 1;
 }
 
 static int init_sig_algs(SSL *s, unsigned int context)

diff --git a/ssl/statem_ntls/statem_lib.c b/ssl/statem_ntls/statem_lib.c
@@ -155,18 +155,15 @@ int tls_setup_handshake_ntls(SSL *s)
 static int get_cert_verify_tbs_data(SSL *s, unsigned char *tls13tbs,
                                     void **hdata, size_t *hdatalen)
 {
-    {
-        size_t retlen;
-        long retlen_l;
+    long retlen_l;
 
-        retlen = retlen_l = BIO_get_mem_data(s->s3->handshake_buffer, hdata);
-        if (retlen_l <= 0) {
-            SSLfatal_ntls(s, SSL_AD_INTERNAL_ERROR, SSL_F_GET_CERT_VERIFY_TBS_DATA,
-                     ERR_R_INTERNAL_ERROR);
-            return 0;
-        }
-        *hdatalen = retlen;
+    retlen_l = BIO_get_mem_data(s->s3->handshake_buffer, hdata);
+    if (retlen_l <= 0) {
+        SSLfatal_ntls(s, SSL_AD_INTERNAL_ERROR, SSL_F_GET_CERT_VERIFY_TBS_DATA,
+                      ERR_R_INTERNAL_ERROR);
+        return 0;
     }
+    *hdatalen = retlen_l;
 
     return 1;
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -3330,7 +3330,6 @@ int s_client_main(int argc, char **argv) @@
             }
         }
-        ret = 0;
      shut:
         if (in_init)
             print_stuff(bio_c_out, con, full_log);
@@ Expand Down @@