Fix coverity issues CID 1457745...1457752, 1457853, 1457854

CID 1457854 - keymgmt_lib.c : OVERRUN
CID 1457853 - self_test_kats.c : UNINT
CID 1457752 - fipsprov.c RESOURCE_LEAK (code change in another PR removed this)
CID 1457751 - apps/pkcs12.c CHECKED_RETURN
CID 1457750 - dsa_ossl.c RESOURCE_LEAK (marked as false positive since tmp can not be NULL)
CID 1457749 - apps/nseq.c : CHECKED_RETURN
CID 1457748 - cipher_aes_cbc_hmac_sha.c : SIZEOF_MISMATCH
CID 1457747 - cipher_aes_cbc_hmac_sha.c : SIZEOF_MISMATCH
CID 1457746 - same as 1457752
CID 1457745 - apps/ocsp  : CHECKED_RETURN

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10934)

diff --git a/apps/nseq.c b/apps/nseq.c
index 5b7ab67dd1ecbcfaafe951a6222a46fa65cc4ac4..9d1e0950e80d8cb4426d7e8135968490f4ea856d 100644 (file)
--- a/apps/nseq.c
+++ b/apps/nseq.c
@@ -82,8 +82,10 @@ int nseq_main(int argc, char **argv)
         seq->certs = sk_X509_new_null();
         if (seq->certs == NULL)
             goto end;
         seq->certs = sk_X509_new_null();
         if (seq->certs == NULL)
             goto end;

-        while ((x509 = PEM_read_bio_X509(in, NULL, NULL, NULL)))
-            sk_X509_push(seq->certs, x509);
+        while ((x509 = PEM_read_bio_X509(in, NULL, NULL, NULL))) {
+            if (!sk_X509_push(seq->certs, x509))
+                goto end;
+        }

 
         if (!sk_X509_num(seq->certs)) {
             BIO_printf(bio_err, "%s: Error reading certs file %s\n",
 
         if (!sk_X509_num(seq->certs)) {
             BIO_printf(bio_err, "%s: Error reading certs file %s\n",

diff --git a/apps/ocsp.c b/apps/ocsp.c
index dc1b7601bb69cd7577496e38e3eab11418a2bf43..4c66e966ef5d8b4a234b54563cd2006fcc1ad64f 100644 (file)
--- a/apps/ocsp.c
+++ b/apps/ocsp.c
@@ -451,7 +451,8 @@ int ocsp_main(int argc, char **argv)
                 if ((issuers = sk_X509_new_null()) == NULL)
                     goto end;
             }
                 if ((issuers = sk_X509_new_null()) == NULL)
                     goto end;
             }

-            sk_X509_push(issuers, issuer);
+            if (!sk_X509_push(issuers, issuer))
+                goto end;

             break;
         case OPT_CERT:
             X509_free(cert);
             break;
         case OPT_CERT:
             X509_free(cert);

diff --git a/apps/pkcs12.c b/apps/pkcs12.c
index 5eff88b64476e984327d1db92875396b863f3bcf..091318b67d8da8a7b2ab2e799045141380547209 100644 (file)
--- a/apps/pkcs12.c
+++ b/apps/pkcs12.c
@@ -893,12 +893,13 @@ static int alg_print(const X509_ALGOR *alg)
 
 int cert_load(BIO *in, STACK_OF(X509) *sk)
 {
 
 int cert_load(BIO *in, STACK_OF(X509) *sk)
 {

-    int ret;
+    int ret = 0;

     X509 *cert;
     X509 *cert;

-    ret = 0;
+

     while ((cert = PEM_read_bio_X509(in, NULL, NULL, NULL))) {
         ret = 1;
     while ((cert = PEM_read_bio_X509(in, NULL, NULL, NULL))) {
         ret = 1;

-        sk_X509_push(sk, cert);
+        if (!sk_X509_push(sk, cert))
+            return 0;

     }
     if (ret)
         ERR_clear_error();
     }
     if (ret)
         ERR_clear_error();

diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index 5e208b21b870793617bcc627190f95e702f6fa3b..6990c0cdaa07a402184150b2f32b8b86ec60bee8 100644 (file)
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -135,7 +135,8 @@ void *evp_keymgmt_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt,
      * have to think about a cache aging scheme, though, if |i| indexes
      * outside the array.
      */
      * have to think about a cache aging scheme, though, if |i| indexes
      * outside the array.
      */

-    j = ossl_assert(i < OSSL_NELEM(pk->pkeys));
+    if (!ossl_assert(i < OSSL_NELEM(pk->pkeys)))
+        return NULL;

 
     evp_keymgmt_cache_pkey(pk, i, keymgmt, provdata, want_domainparams);
 
 
     evp_keymgmt_cache_pkey(pk, i, keymgmt, provdata, want_domainparams);
 

diff --git a/providers/fips/self_test_kats.c b/providers/fips/self_test_kats.c
index 3ccd3f66ed649941997e2a403c9c4c9307306005..f67f4f69c8fb3c93a8a0d331ec737eba5b54fa23 100644 (file)
--- a/providers/fips/self_test_kats.c
+++ b/providers/fips/self_test_kats.c
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright 2019 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2019-2020 The OpenSSL Project Authors. All Rights Reserved.

  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy


@@ -10,6 +10,7 @@
 #include <string.h>
 #include <openssl/evp.h>
 #include <openssl/kdf.h>
 #include <string.h>
 #include <openssl/evp.h>
 #include <openssl/kdf.h>

+#include "internal/cryptlib.h"

 #include "internal/nelem.h"
 #include "self_test.h"
 #include "self_test_data.inc"
 #include "internal/nelem.h"
 #include "self_test.h"
 #include "self_test_data.inc"


@@ -140,15 +141,20 @@ static int self_test_kdf(const ST_KAT_KDF *t, OSSL_ST_EVENT *event,
                          OPENSSL_CTX *libctx)
 {
     int ret = 0;
                          OPENSSL_CTX *libctx)
 {
     int ret = 0;

-    int i;
+    int i, numparams;

     unsigned char out[64];
     EVP_KDF *kdf = NULL;
     EVP_KDF_CTX *ctx = NULL;
     OSSL_PARAM params[16];
     const OSSL_PARAM *settables = NULL;
 
     unsigned char out[64];
     EVP_KDF *kdf = NULL;
     EVP_KDF_CTX *ctx = NULL;
     OSSL_PARAM params[16];
     const OSSL_PARAM *settables = NULL;
 

+    numparams = OSSL_NELEM(params);

     SELF_TEST_EVENT_onbegin(event, OSSL_SELF_TEST_TYPE_KAT_KDF, t->desc);
 
     SELF_TEST_EVENT_onbegin(event, OSSL_SELF_TEST_TYPE_KAT_KDF, t->desc);
 

+    /* Zeroize the params array to avoid mem leaks on error */
+    for (i = 0; i < numparams; ++i)
+        params[i] = OSSL_PARAM_construct_end();
+

     kdf = EVP_KDF_fetch(libctx, t->algorithm, "");
     ctx = EVP_KDF_CTX_new(kdf);
     if (ctx == NULL)
     kdf = EVP_KDF_fetch(libctx, t->algorithm, "");
     ctx = EVP_KDF_CTX_new(kdf);
     if (ctx == NULL)


@@ -156,13 +162,14 @@ static int self_test_kdf(const ST_KAT_KDF *t, OSSL_ST_EVENT *event,
 
     settables = EVP_KDF_settable_ctx_params(kdf);
     for (i = 0; t->ctrls[i].name != NULL; ++i) {
 
     settables = EVP_KDF_settable_ctx_params(kdf);
     for (i = 0; t->ctrls[i].name != NULL; ++i) {

+        if (!ossl_assert(i < (numparams - 1)))
+            goto end;

         if (!OSSL_PARAM_allocate_from_text(&params[i], settables,
                                            t->ctrls[i].name,
                                            t->ctrls[i].value,
                                            strlen(t->ctrls[i].value)))
             goto end;
     }
         if (!OSSL_PARAM_allocate_from_text(&params[i], settables,
                                            t->ctrls[i].name,
                                            t->ctrls[i].value,
                                            strlen(t->ctrls[i].value)))
             goto end;
     }

-    params[i] = OSSL_PARAM_construct_end();

     if (!EVP_KDF_CTX_set_params(ctx, params))
         goto end;
 
     if (!EVP_KDF_CTX_set_params(ctx, params))
         goto end;
 

diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
index 6af46ce2aaa9ba323f51aafa27f2c8aadf98fdcf..f6f4cb339dd8f7ba2d772078c5d5e4d7bd822bb5 100644 (file)
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
@@ -280,7 +280,7 @@ static void aes_cbc_hmac_sha1_freectx(void *vctx)
     PROV_AES_HMAC_SHA1_CTX *ctx = (PROV_AES_HMAC_SHA1_CTX *)vctx;
 
     if (ctx != NULL)
     PROV_AES_HMAC_SHA1_CTX *ctx = (PROV_AES_HMAC_SHA1_CTX *)vctx;
 
     if (ctx != NULL)

-        OPENSSL_clear_free(ctx, sizeof(ctx));
+        OPENSSL_clear_free(ctx, sizeof(*ctx));

 }
 
 static void *aes_cbc_hmac_sha256_newctx(void *provctx, size_t kbits,
 }
 
 static void *aes_cbc_hmac_sha256_newctx(void *provctx, size_t kbits,


@@ -301,7 +301,7 @@ static void aes_cbc_hmac_sha256_freectx(void *vctx)
     PROV_AES_HMAC_SHA256_CTX *ctx = (PROV_AES_HMAC_SHA256_CTX *)vctx;
 
     if (ctx != NULL)
     PROV_AES_HMAC_SHA256_CTX *ctx = (PROV_AES_HMAC_SHA256_CTX *)vctx;
 
     if (ctx != NULL)

-        OPENSSL_clear_free(ctx, sizeof(ctx));
+        OPENSSL_clear_free(ctx, sizeof(*ctx));

 }
 
 # define IMPLEMENT_CIPHER(nm, sub, kbits, blkbits, ivbits, flags)               \
 }
 
 # define IMPLEMENT_CIPHER(nm, sub, kbits, blkbits, ivbits, flags)               \