Merge pull request #635 from dongbeiouba/fix84/coverity

Fix coverity issues for branch 8.4-stable

apps/speed.c

@@ -4883,6 +4883,7 @@ static int do_multi(int multi, int size_num)
             close(1);
             if (dup(fd[1]) == -1) {
                 BIO_printf(bio_err, "dup failed\n");
+                close(fd[1]);
                 exit(1);
             }
             close(fd[1]);

crypto/dh/dh_pmeth.c

@@ -432,7 +432,7 @@ static int pkey_dh_derive(EVP_PKEY_CTX *ctx, unsigned char *key,
     else if (dctx->kdf_type == EVP_PKEY_DH_KDF_X9_42) {
 
         unsigned char *Z = NULL;
-        size_t Zlen = 0;
+        int Zlen = 0;
         if (!dctx->kdf_outlen || !dctx->kdf_oid)
             return 0;
         if (key == NULL) {

crypto/ec/ec_elgamal_crypt.c

@@ -199,7 +199,7 @@ EC_ELGAMAL_MR_CTX *EC_ELGAMAL_MR_CTX_new(STACK_OF(EC_KEY) *keys, const EC_POINT
 
         if (h != NULL) {
             if (!(ctx->h = EC_POINT_dup(h, ctx->group)))
-                return 0;
+                goto err;
         } else {
             ctx->h = EC_POINT_new(group);
             if (ctx->h == NULL) {

crypto/ec/ec_lib.c

@@ -795,8 +795,10 @@ EC_POINTS *EC_POINTS_new(const EC_GROUP *group, int count)
 
     for (i = 0; i < count; i++) {
         point = EC_POINT_new(group);
-        if (point == NULL)
+        if (point == NULL) {
             EC_POINTS_free(ret);
+            return NULL;
+        }
 
         ret->items[i] = point;
     }
@@ -1544,13 +1546,13 @@ int EC_POINTs_from_strings(const EC_GROUP *group, EC_POINTS **r,
 {
     int ret = 0;
     BN_CTX *new_ctx = NULL;
-    EC_POINTS *result = NULL;
+    EC_POINTS *result = NULL, *new_r = NULL;
 
     if (r == NULL || strings == NULL || num <= 0)
         return 0;
 
     if (*r == NULL) {
-        result = EC_POINTS_new(group, (uint32_t)num);
+        new_r = result = EC_POINTS_new(group, (uint32_t)num);
         if (result == NULL)
             return 0;
     } else {
@@ -1574,14 +1576,13 @@ int EC_POINTs_from_strings(const EC_GROUP *group, EC_POINTS **r,
     }
 #endif
 
-    result = NULL;
-
     /*
      * TODO
      */
 err:
     BN_CTX_free(new_ctx);
-    EC_POINTS_free(result);
+    if (*r != new_r)
+        EC_POINTS_free(new_r);
     return ret;
 }
 
@@ -1600,13 +1601,13 @@ int EC_POINTs_from_strings_scalar_mul(const EC_GROUP *group, EC_POINTS **r,
 {
     int ret = 0;
     BN_CTX *new_ctx = NULL;
-    EC_POINTS *result = NULL;
+    EC_POINTS *result, *new_r = NULL;
 
     if (r == NULL || strings == NULL || num <= 0)
         return 0;
 
     if (*r == NULL) {
-        result = EC_POINTS_new(group, (uint32_t)num);
+        new_r = result = EC_POINTS_new(group, (uint32_t)num);
         if (result == NULL)
             return 0;
     } else {
@@ -1632,14 +1633,13 @@ int EC_POINTs_from_strings_scalar_mul(const EC_GROUP *group, EC_POINTS **r,
     }
 #endif
 
-    result = NULL;
-
     /*
      * TODO
      */
 err:
     BN_CTX_free(new_ctx);
-    EC_POINTS_free(result);
+    if (*r != new_r)
+        EC_POINTS_free(new_r);
     return ret;
 }
 

crypto/evp/e_des.c

@@ -135,7 +135,8 @@ static int des_cfb1_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
                            const unsigned char *in, size_t inl)
 {
     size_t n, chunk = EVP_MAXCHUNK / 8;
-    unsigned char c[1], d[1];
+    unsigned char c[1];
+    unsigned char d[1] = { 0 };
 
     if (inl < chunk)
         chunk = inl;

crypto/evp/e_des3.c

@@ -151,7 +151,8 @@ static int des_ede3_cfb1_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
                                 const unsigned char *in, size_t inl)
 {
     size_t n;
-    unsigned char c[1], d[1];
+    unsigned char c[1];
+    unsigned char d[1] = { 0 };
 
     if (!EVP_CIPHER_CTX_test_flags(ctx, EVP_CIPH_FLAG_LENGTH_BITS))
             inl *= 8;

crypto/packet.c

@@ -207,7 +207,7 @@ int WPACKET_set_flags(WPACKET *pkt, unsigned int flags)
 }
 
 /* Store the |value| of length |len| at location |data| */
-static int put_value(unsigned char *data, size_t value, size_t len)
+static int put_value(unsigned char *data, uint64_t value, size_t len)
 {
     if (data == NULL)
         return 1;
@@ -379,12 +379,12 @@ int WPACKET_start_sub_packet(WPACKET *pkt)
     return WPACKET_start_sub_packet_len__(pkt, 0);
 }
 
-int WPACKET_put_bytes__(WPACKET *pkt, unsigned int val, size_t size)
+int WPACKET_put_bytes__(WPACKET *pkt, uint64_t val, size_t size)
 {
     unsigned char *data;
 
     /* Internal API, so should not fail */
-    if (!ossl_assert(size <= sizeof(unsigned int))
+    if (!ossl_assert(size <= sizeof(uint64_t))
             || !WPACKET_allocate_bytes(pkt, size, &data)
             || !put_value(data, val, size))
         return 0;

crypto/zkp/bulletproofs/bulletproofs_encode.c

@@ -184,8 +184,10 @@ static int bp_inner_product_proof_encode(bp_inner_product_proof_t *ip_proof,
 
     len += sk_len;
 
-    if (out == NULL)
+    if (out == NULL) {
+        sk_BIGNUM_free(sk_bn);
         return len;
+    }
 
     sk_len = zkp_stack_of_bignum_encode(sk_bn, p, bn_len);
     if (sk_len == 0)

crypto/zkp/gadget/zkp_range_proof.c

@@ -410,6 +410,7 @@ void ZKP_RANGE_PROOF_free(ZKP_RANGE_PROOF *proof)
 
     NIZK_PLAINTEXT_KNOWLEDGE_PROOF_free(proof->ptke_proof);
     BP_RANGE_PROOF_free(proof->bp_proof);
+    OPENSSL_free(proof);
 }
 
 ZKP_RANGE_PROOF *ZKP_RANGE_PROOF_prove(ZKP_RANGE_CTX *ctx, int left_bound_bits,

crypto/zkp/nizk/nizk_encode.c

@@ -296,7 +296,7 @@ NIZK_WITNESS *NIZK_WITNESS_decode(const unsigned char *in, size_t size, int flag
     if (flag == 1) {
         if (size < (sizeof(int) + bn_len * 3)) {
             ERR_raise(ERR_LIB_ZKP_NIZK, ERR_R_PASSED_INVALID_ARGUMENT);
-            return NULL;
+            goto err;
         }
 
         witness->v = zkp_bignum_decode(p, NULL, bn_len);

include/internal/packet.h

@@ -228,6 +228,28 @@ __owur static ossl_inline int PACKET_peek_net_4(const PACKET *pkt,
     return 1;
 }
 
+/*
+ * Peek ahead at 8 bytes in network order from |pkt| and store the value in
+ * |*data|
+ */
+__owur static ossl_inline int PACKET_peek_net_8(const PACKET *pkt,
+                                                uint64_t *data)
+{
+    if (PACKET_remaining(pkt) < 8)
+        return 0;
+
+    *data = ((uint64_t)(*pkt->curr)) << 56;
+    *data |= ((uint64_t)(*(pkt->curr + 1))) << 48;
+    *data |= ((uint64_t)(*(pkt->curr + 2))) << 40;
+    *data |= ((uint64_t)(*(pkt->curr + 3))) << 32;
+    *data |= ((uint64_t)(*(pkt->curr + 4))) << 24;
+    *data |= ((uint64_t)(*(pkt->curr + 5))) << 16;
+    *data |= ((uint64_t)(*(pkt->curr + 6))) << 8;
+    *data |= *(pkt->curr + 7);
+
+    return 1;
+}
+
 /* Equivalent of n2l */
 /* Get 4 bytes in network order from |pkt| and store the value in |*data| */
 __owur static ossl_inline int PACKET_get_net_4(PACKET *pkt, unsigned long *data)
@@ -252,6 +274,17 @@ __owur static ossl_inline int PACKET_get_net_4_len(PACKET *pkt, size_t *data)
     return ret;
 }
 
+/* Get 8 bytes in network order from |pkt| and store the value in |*data| */
+__owur static ossl_inline int PACKET_get_net_8(PACKET *pkt, uint64_t *data)
+{
+    if (!PACKET_peek_net_8(pkt, data))
+        return 0;
+
+    packet_forward(pkt, 8);
+
+    return 1;
+}
+
 /* Peek ahead at 1 byte from |pkt| and store the value in |*data| */
 __owur static ossl_inline int PACKET_peek_1(const PACKET *pkt,
                                             unsigned int *data)
@@ -833,7 +866,7 @@ int WPACKET_sub_reserve_bytes__(WPACKET *pkt, size_t len,
  * 1 byte will fail. Don't call this directly. Use the convenience macros below
  * instead.
  */
-int WPACKET_put_bytes__(WPACKET *pkt, unsigned int val, size_t bytes);
+int WPACKET_put_bytes__(WPACKET *pkt, uint64_t val, size_t bytes);
 
 /*
  * Convenience macros for calling WPACKET_put_bytes with different
@@ -847,6 +880,8 @@ int WPACKET_put_bytes__(WPACKET *pkt, unsigned int val, size_t bytes);
     WPACKET_put_bytes__((pkt), (val), 3)
 #define WPACKET_put_bytes_u32(pkt, val) \
     WPACKET_put_bytes__((pkt), (val), 4)
+#define WPACKET_put_bytes_u64(pkt, val) \
+    WPACKET_put_bytes__((pkt), (val), 8)
 
 /* Set a maximum size that we will not allow the WPACKET to grow beyond */
 int WPACKET_set_max_size(WPACKET *pkt, size_t maxsize);

providers/implementations/ciphers/cipher_des_hw.c

@@ -136,7 +136,8 @@ static int cipher_hw_des_cfb1_cipher(PROV_CIPHER_CTX *ctx, unsigned char *out,
 {
     size_t n, chunk = MAXCHUNK / 8;
     DES_key_schedule *key = &(((PROV_DES_CTX *)ctx)->dks.ks);
-    unsigned char c[1], d[1];
+    unsigned char c[1];
+    unsigned char d[1] = { 0 };
 
     if (inl < chunk)
         chunk = inl;

providers/implementations/ciphers/cipher_tdes_default_hw.c

@@ -99,7 +99,8 @@ static int ossl_cipher_hw_tdes_cfb1(PROV_CIPHER_CTX *ctx, unsigned char *out,
 {
     PROV_TDES_CTX *tctx = (PROV_TDES_CTX *)ctx;
     size_t n;
-    unsigned char c[1], d[1];
+    unsigned char c[1];
+    unsigned char d[1] = { 0 };
 
     if (ctx->use_bits == 0)
         inl *= 8;

ssl/statem/extensions_clnt.c

@@ -1038,7 +1038,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
                                   X509 *x, size_t chainidx)
 {
 #ifndef OPENSSL_NO_TLS1_3
-    uint32_t now, agesec, agems = 0;
+    uint32_t agesec, agems = 0;
     size_t reshashsize = 0, pskhashsize = 0, binderoffset, msglen;
     unsigned char *resbinder = NULL, *pskbinder = NULL, *msgstart = NULL;
     const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL;
@@ -1094,8 +1094,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
          * this in multiple places in the code, so portability shouldn't be an
          * issue.
          */
-        now = (uint32_t)time(NULL);
-        agesec = now - (uint32_t)s->session->time;
+        agesec = (uint32_t)(time(NULL) - s->session->time);
         /*
          * We calculate the age in seconds but the server may work in ms. Due to
          * rounding errors we could overestimate the age by up to 1s. It is

ssl/statem/extensions_srvr.c

@@ -12,16 +12,16 @@
 #include "statem_local.h"
 #include "internal/cryptlib.h"
 
-#define COOKIE_STATE_FORMAT_VERSION     0
+#define COOKIE_STATE_FORMAT_VERSION     1
 
 /*
  * 2 bytes for packet length, 2 bytes for format version, 2 bytes for
  * protocol version, 2 bytes for group id, 2 bytes for cipher id, 1 byte for
- * key_share present flag, 4 bytes for timestamp, 2 bytes for the hashlen,
+ * key_share present flag, 8 bytes for timestamp, 2 bytes for the hashlen,
  * EVP_MAX_MD_SIZE for transcript hash, 1 byte for app cookie length, app cookie
  * length bytes, SHA256_DIGEST_LENGTH bytes for the HMAC of the whole thing.
  */
-#define MAX_COOKIE_SIZE (2 + 2 + 2 + 2 + 2 + 1 + 4 + 2 + EVP_MAX_MD_SIZE + 1 \
+#define MAX_COOKIE_SIZE (2 + 2 + 2 + 2 + 2 + 1 + 8 + 2 + EVP_MAX_MD_SIZE + 1 \
                          + SSL_COOKIE_LENGTH + SHA256_DIGEST_LENGTH)
 
 /*
@@ -694,7 +694,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     unsigned char hmac[SHA256_DIGEST_LENGTH];
     unsigned char hrr[MAX_HRR_SIZE];
     size_t rawlen, hmaclen, hrrlen, ciphlen;
-    unsigned long tm, now;
+    uint64_t tm, now;
 
     /* Ignore any cookie if we're not set up to verify it */
     if (s->ctx->verify_stateless_cookie_cb == NULL
@@ -795,7 +795,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     }
 
     if (!PACKET_get_1(&cookie, &key_share)
-            || !PACKET_get_net_4(&cookie, &tm)
+            || !PACKET_get_net_8(&cookie, &tm)
             || !PACKET_get_length_prefixed_2(&cookie, &chhash)
             || !PACKET_get_length_prefixed_1(&cookie, &appcookie)
             || PACKET_remaining(&cookie) != SHA256_DIGEST_LENGTH) {
@@ -804,7 +804,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     }
 
     /* We tolerate a cookie age of up to 10 minutes (= 60 * 10 seconds) */
-    now = (unsigned long)time(NULL);
+    now = time(NULL);
     if (tm > now || (now - tm) > 600) {
         /* Cookie is stale. Ignore it */
         return 1;
@@ -1135,7 +1135,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                 s->ext.early_data_ok = 1;
             s->ext.ticket_expected = 1;
         } else {
-            uint32_t ticket_age = 0, now, agesec, agems;
+            uint32_t ticket_age = 0, agesec, agems;
             int ret;
 
             /*
@@ -1175,8 +1175,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
             }
 
             ticket_age = (uint32_t)ticket_agel;
-            now = (uint32_t)time(NULL);
-            agesec = now - (uint32_t)sess->time;
+            agesec = (uint32_t)(time(NULL) - sess->time);
             agems = agesec * (uint32_t)1000;
             ticket_age -= sess->ext.tick_age_add;
 
@@ -1856,7 +1855,7 @@ EXT_RETURN tls_construct_stoc_cookie(SSL *s, WPACKET *pkt, unsigned int context,
                                               &ciphlen)
                /* Is there a key_share extension present in this HRR? */
             || !WPACKET_put_bytes_u8(pkt, s->s3.peer_tmp == NULL)
-            || !WPACKET_put_bytes_u32(pkt, (unsigned int)time(NULL))
+            || !WPACKET_put_bytes_u64(pkt, time(NULL))
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_reserve_bytes(pkt, EVP_MAX_MD_SIZE, &hashval1)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);