From d5e66eab0bc08d701ba8386d3a36d417d19966aa Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Tue, 4 Feb 2020 13:50:51 +1000 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/10934) --- apps/nseq.c | 6 ++++-- apps/ocsp.c | 3 ++- apps/pkcs12.c | 7 ++++--- crypto/evp/keymgmt_lib.c | 3 ++- providers/fips/self_test_kats.c | 13 ++++++++++--- .../ciphers/cipher_aes_cbc_hmac_sha.c | 4 ++-- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/apps/nseq.c b/apps/nseq.c index 5b7ab67dd1..9d1e0950e8 100644 --- 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; - 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", diff --git a/apps/ocsp.c b/apps/ocsp.c index dc1b7601bb..4c66e966ef 100644 --- 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; } - sk_X509_push(issuers, issuer); + if (!sk_X509_push(issuers, issuer)) + goto end; break; case OPT_CERT: X509_free(cert); diff --git a/apps/pkcs12.c b/apps/pkcs12.c index 5eff88b644..091318b67d 100644 --- 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 ret; + int ret = 0; X509 *cert; - ret = 0; + 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(); diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c index 5e208b21b8..6990c0cdaa 100644 --- 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. */ - 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); diff --git a/providers/fips/self_test_kats.c b/providers/fips/self_test_kats.c index 3ccd3f66ed..f67f4f69c8 100644 --- 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 @@ -10,6 +10,7 @@ #include #include #include +#include "internal/cryptlib.h" #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; - 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; + numparams = OSSL_NELEM(params); 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) @@ -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) { + if (!ossl_assert(i < (numparams - 1))) + goto end; if (!OSSL_PARAM_allocate_from_text(¶ms[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; diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c index 6af46ce2aa..f6f4cb339d 100644 --- 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) - OPENSSL_clear_free(ctx, sizeof(ctx)); + OPENSSL_clear_free(ctx, sizeof(*ctx)); } 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) - OPENSSL_clear_free(ctx, sizeof(ctx)); + OPENSSL_clear_free(ctx, sizeof(*ctx)); } # define IMPLEMENT_CIPHER(nm, sub, kbits, blkbits, ivbits, flags) \ -- 2.39.2