From 3101ab603cd82cdbc81de0902b2b4718e8f1279b Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 3 Sep 2020 11:50:30 +0100 Subject: [PATCH] Fix an EVP_MD_CTX leak If we initialise an EVP_MD_CTX with a legacy MD, and then reuse the same EVP_MD_CTX with a provided MD then we end up leaking the md_data. We need to ensure we free the md_data if we change to a provided MD. Reviewed-by: Tomas Mraz Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/12779) --- crypto/evp/digest.c | 45 ++++++++++++++++++++++++------------------- crypto/evp/m_sigver.c | 10 ++++++++++ include/crypto/evp.h | 2 ++ 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c index 19fddb74ab..07bf12e5ae 100644 --- a/crypto/evp/digest.c +++ b/crypto/evp/digest.c @@ -22,24 +22,9 @@ #include "internal/provider.h" #include "evp_local.h" -/* This call frees resources associated with the context */ -int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) -{ - if (ctx == NULL) - return 1; - -#ifndef FIPS_MODULE - /* TODO(3.0): Temporarily no support for EVP_DigestSign* in FIPS module */ - /* - * pctx should be freed by the user of EVP_MD_CTX - * if EVP_MD_CTX_FLAG_KEEP_PKEY_CTX is set - */ - if (!EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_KEEP_PKEY_CTX)) { - EVP_PKEY_CTX_free(ctx->pctx); - ctx->pctx = NULL; - } -#endif +void evp_md_ctx_clear_digest(EVP_MD_CTX *ctx, int force) +{ EVP_MD_free(ctx->fetched_digest); ctx->fetched_digest = NULL; ctx->reqdigest = NULL; @@ -61,16 +46,36 @@ int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_CLEANED)) ctx->digest->cleanup(ctx); if (ctx->digest && ctx->digest->ctx_size && ctx->md_data - && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE)) { + && (!EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE) || force)) OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size); - } + if (force) + ctx->digest = NULL; #if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE) ENGINE_finish(ctx->engine); + ctx->engine = NULL; #endif +} - /* TODO(3.0): End of legacy code */ +/* This call frees resources associated with the context */ +int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) +{ + if (ctx == NULL) + return 1; + +#ifndef FIPS_MODULE + /* TODO(3.0): Temporarily no support for EVP_DigestSign* in FIPS module */ + /* + * pctx should be freed by the user of EVP_MD_CTX + * if EVP_MD_CTX_FLAG_KEEP_PKEY_CTX is set + */ + if (!EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_KEEP_PKEY_CTX)) { + EVP_PKEY_CTX_free(ctx->pctx); + ctx->pctx = NULL; + } +#endif + evp_md_ctx_clear_digest(ctx, 0); OPENSSL_cleanse(ctx, sizeof(*ctx)); return 1; diff --git a/crypto/evp/m_sigver.c b/crypto/evp/m_sigver.c index a60d6e770b..e2bb613a20 100644 --- a/crypto/evp/m_sigver.c +++ b/crypto/evp/m_sigver.c @@ -176,6 +176,12 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx, } if (mdname != NULL) { + /* + * We're about to get a new digest so clear anything associated with + * an old digest. + */ + evp_md_ctx_clear_digest(ctx, 1); + /* * This might be requested by a later call to EVP_MD_CTX_md(). * In that case the "explicit fetch" rules apply for that @@ -185,6 +191,10 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx, */ ctx->digest = ctx->reqdigest = ctx->fetched_digest = EVP_MD_fetch(locpctx->libctx, mdname, props); + if (ctx->digest == NULL) { + ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR); + goto err; + } } } diff --git a/include/crypto/evp.h b/include/crypto/evp.h index 9d9b0a7298..bdff97f639 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -808,3 +808,5 @@ int evp_pkey_ctx_use_cached_data(EVP_PKEY_CTX *ctx); void evp_method_store_flush(OPENSSL_CTX *libctx); int evp_set_default_properties_int(OPENSSL_CTX *libctx, const char *propq, int loadconfig); + +void evp_md_ctx_clear_digest(EVP_MD_CTX *ctx, int force); -- 2.39.2