The branch master has been updated via 5e8cd0a4f48f06df2542e7c74dcbb4310bce7c4c (commit) via 97f7a6d42e9d2f38969aff75b0d8f00d934ac8cf (commit) via 9951eaf467f8cc43ffad69222b42340c3b24cd52 (commit) via 0f0b7dfbe5740da9e3fdf4753f8fb56301af13b7 (commit) via 6e417f951c64f4643cdc62c370badf46d5fe485e (commit) via 34816949460e7131af4de421806845be213354d4 (commit) from c1aba0763c477f345c065007ff6295dbe6ec4f64 (commit)
- Log ----------------------------------------------------------------- commit 5e8cd0a4f48f06df2542e7c74dcbb4310bce7c4c Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 18:45:39 2020 +1000 Fix coverity issue: CID 1466479 - Resource leak in apps/pkcs12.c Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) commit 97f7a6d42e9d2f38969aff75b0d8f00d934ac8cf Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 18:21:46 2020 +1000 Fix coverity issue: CID 1466482 - Resource leak in OSSL_STORE_SEARCH_by_key_fingerprint() Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) commit 9951eaf467f8cc43ffad69222b42340c3b24cd52 Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 18:19:13 2020 +1000 Fix coverity issue: CID 1466483 - Improper use of Negative value in dh_ctrl.c Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) commit 0f0b7dfbe5740da9e3fdf4753f8fb56301af13b7 Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 17:30:02 2020 +1000 Fix coverity issue: CID 1466484 - Remove dead code in PKCS7_dataInit() Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) commit 6e417f951c64f4643cdc62c370badf46d5fe485e Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 17:22:40 2020 +1000 Fix coverity issue: CID 1466485 - Explicit NULL dereference in OSSL_STORE_find() Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) commit 34816949460e7131af4de421806845be213354d4 Author: Shane Lontis <shane.lon...@oracle.com> Date: Thu Sep 10 16:40:24 2020 +1000 Fix coverity issue: CID 1466486 - Resource leak in OSSL_STORE Note that although this is a false positive currently, it could become possible if any of the methods called change behaviour - so it is safer to add the fix than to ignore it. Added a simple test so that I could prove this was the case. Reviewed-by: Tomas Mraz <tm...@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/12847) ----------------------------------------------------------------------- Summary of changes: apps/pkcs12.c | 8 ++- crypto/dh/dh_ctrl.c | 3 + crypto/pkcs7/pk7_doit.c | 7 +- crypto/store/store_lib.c | 6 ++ test/build.info | 6 +- test/ossl_store_test.c | 84 ++++++++++++++++++++++ .../{04-test_hexstring.t => 66-test_ossl_store.t} | 10 ++- 7 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 test/ossl_store_test.c copy test/recipes/{04-test_hexstring.t => 66-test_ossl_store.t} (67%) diff --git a/apps/pkcs12.c b/apps/pkcs12.c index f5bb18a8db..23ffa98f77 100644 --- a/apps/pkcs12.c +++ b/apps/pkcs12.c @@ -541,13 +541,15 @@ int pkcs12_main(int argc, char **argv) X509_STORE_free(store); if (vret == X509_V_OK) { + int add_certs; /* Remove from chain2 the first (end entity) certificate */ X509_free(sk_X509_shift(chain2)); /* Add the remaining certs (except for duplicates) */ - if (!X509_add_certs(certs, chain2, X509_ADD_FLAG_UP_REF - | X509_ADD_FLAG_NO_DUP)) - goto export_end; + add_certs = X509_add_certs(certs, chain2, X509_ADD_FLAG_UP_REF + | X509_ADD_FLAG_NO_DUP); sk_X509_pop_free(chain2, X509_free); + if (!add_certs) + goto export_end; } else { if (vret != X509_V_ERR_UNSPECIFIED) BIO_printf(bio_err, "Error getting chain: %s\n", diff --git a/crypto/dh/dh_ctrl.c b/crypto/dh/dh_ctrl.c index 6fddd271a8..0db5eba505 100644 --- a/crypto/dh/dh_ctrl.c +++ b/crypto/dh/dh_ctrl.c @@ -500,6 +500,9 @@ int EVP_PKEY_CTX_set0_dh_kdf_ukm(EVP_PKEY_CTX *ctx, unsigned char *ukm, int len) int ret; OSSL_PARAM params[2], *p = params; + if (len <= 0) + return -1; + ret = dh_param_derive_check(ctx); if (ret != 1) return ret; diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c index 6a7af7826d..c48c629398 100644 --- a/crypto/pkcs7/pk7_doit.c +++ b/crypto/pkcs7/pk7_doit.c @@ -316,17 +316,12 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) fetched_cipher = EVP_CIPHER_fetch(p7_ctx->libctx, EVP_CIPHER_name(evp_cipher), p7_ctx->propq); + (void)ERR_pop_to_mark(); if (fetched_cipher != NULL) cipher = fetched_cipher; else cipher = evp_cipher; - if (cipher == NULL) { - (void)ERR_clear_last_mark(); - goto err; - } - (void)ERR_pop_to_mark(); - if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, 1) <= 0) goto err; diff --git a/crypto/store/store_lib.c b/crypto/store/store_lib.c index 61558a9b6e..0f686fb119 100644 --- a/crypto/store/store_lib.c +++ b/crypto/store/store_lib.c @@ -178,6 +178,7 @@ OSSL_STORE_open_with_libctx(const char *uri, } OSSL_STORE_LOADER_free(fetched_loader); OPENSSL_free(propq_copy); + OPENSSL_free(ctx); return NULL; } @@ -271,6 +272,10 @@ int OSSL_STORE_find(OSSL_STORE_CTX *ctx, const OSSL_STORE_SEARCH *search) ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_LOADING_STARTED); return 0; } + if (search == NULL) { + ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_PASSED_NULL_PARAMETER); + return 0; + } if (ctx->fetched_loader != NULL) { OSSL_PARAM_BLD *bld; @@ -845,6 +850,7 @@ OSSL_STORE_SEARCH *OSSL_STORE_SEARCH_by_key_fingerprint(const EVP_MD *digest, OSSL_STORE_R_FINGERPRINT_SIZE_DOES_NOT_MATCH_DIGEST, "%s size is %d, fingerprint size is %zu", EVP_MD_name(digest), EVP_MD_size(digest), len); + OPENSSL_free(search); return NULL; } diff --git a/test/build.info b/test/build.info index 7c80b16284..0b67d49b38 100644 --- a/test/build.info +++ b/test/build.info @@ -36,7 +36,7 @@ IF[{- !$disabled{tests} -}] destest mdc2test \ exptest \ evp_pkey_provided_test evp_test evp_extra_test evp_extra_test2 \ - evp_fetch_prov_test acvp_test evp_libctx_test \ + evp_fetch_prov_test acvp_test evp_libctx_test ossl_store_test \ v3nametest v3ext \ evp_pkey_provided_test evp_test evp_extra_test evp_extra_test2 \ evp_fetch_prov_test v3nametest v3ext \ @@ -166,6 +166,10 @@ IF[{- !$disabled{tests} -}] DEPEND[acvp_test]=../libcrypto.a libtestutil.a ENDIF + SOURCE[ossl_store_test]=ossl_store_test.c + INCLUDE[ossl_store_test]=../include ../apps/include + DEPEND[ossl_store_test]=../libcrypto.a libtestutil.a + SOURCE[provider_status_test]=provider_status_test.c INCLUDE[provider_status_test]=../include ../apps/include DEPEND[provider_status_test]=../libcrypto.a libtestutil.a diff --git a/test/ossl_store_test.c b/test/ossl_store_test.c new file mode 100644 index 0000000000..2ce7d51877 --- /dev/null +++ b/test/ossl_store_test.c @@ -0,0 +1,84 @@ +/* + * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the Apache License 2.0 (the "License"). You may not use + * this file except in compliance with the License. You can obtain a copy + * in the file LICENSE in the source distribution or at + * https://www.openssl.org/source/license.html + */ + +#include <openssl/store.h> +#include <openssl/ui.h> +#include "testutil.h" + +typedef enum OPTION_choice { + OPT_ERR = -1, + OPT_EOF = 0, + OPT_INFILE, + OPT_TEST_ENUM +} OPTION_CHOICE; + +static const char *infile = NULL; + +static int test_store_open(void) +{ + int ret = 0; + OSSL_STORE_CTX *sctx = NULL; + OSSL_STORE_SEARCH *search = NULL; + UI_METHOD *ui_method = NULL; + + ret = TEST_ptr(search = OSSL_STORE_SEARCH_by_alias("nothing")) + && TEST_ptr(ui_method= UI_create_method("DummyUI")) + && TEST_ptr(sctx = OSSL_STORE_open_with_libctx(infile, NULL, NULL, + ui_method, NULL, + NULL, NULL)) + && TEST_false(OSSL_STORE_find(sctx, NULL)) + && TEST_true(OSSL_STORE_find(sctx, search)); + UI_destroy_method(ui_method); + OSSL_STORE_SEARCH_free(search); + OSSL_STORE_close(sctx); + return ret; +} + +static int test_store_search_by_key_fingerprint_fail(void) +{ + int ret; + OSSL_STORE_SEARCH *search = NULL; + + ret = TEST_ptr_null(search = OSSL_STORE_SEARCH_by_key_fingerprint( + EVP_sha256(), NULL, 0)); + OSSL_STORE_SEARCH_free(search); + return ret; +} + +const OPTIONS *test_get_options(void) +{ + static const OPTIONS test_options[] = { + OPT_TEST_OPTIONS_DEFAULT_USAGE, + { "in", OPT_INFILE, '<', }, + { NULL } + }; + return test_options; +} + +int setup_tests(void) +{ + OPTION_CHOICE o; + + while ((o = opt_next()) != OPT_EOF) { + switch (o) { + case OPT_INFILE: + infile = opt_arg(); + break; + case OPT_TEST_CASES: + break; + default: + case OPT_ERR: + return 0; + } + } + + ADD_TEST(test_store_open); + ADD_TEST(test_store_search_by_key_fingerprint_fail); + return 1; +} diff --git a/test/recipes/04-test_hexstring.t b/test/recipes/66-test_ossl_store.t similarity index 67% copy from test/recipes/04-test_hexstring.t copy to test/recipes/66-test_ossl_store.t index 664868fe60..634b0e76a8 100644 --- a/test/recipes/04-test_hexstring.t +++ b/test/recipes/66-test_ossl_store.t @@ -7,9 +7,13 @@ # https://www.openssl.org/source/license.html use strict; -use OpenSSL::Test; +use warnings; + use OpenSSL::Test::Simple; +use OpenSSL::Test qw/:DEFAULT srctop_file/; + +setup("test_ossl_store"); -setup("test_hexstring"); +plan tests => 1; -simple_test("test_hexstring", "hexstr_test"); +ok(run(test(["ossl_store_test", "-in", srctop_file("test", "testrsa.pem")])));