From: Bernd Edlinger Date: Thu, 7 Sep 2023 16:05:44 +0000 (+0200) Subject: Fix error handling in CMS_EncryptedData_encrypt X-Git-Tag: openssl-3.1.8~71 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8d723690b0863cafcbae206acf48f9a8d3115fd0;p=thirdparty%2Fopenssl.git Fix error handling in CMS_EncryptedData_encrypt That caused several memory leaks in case of error. Also when the CMS object that is created by CMS_EncryptedData_encrypt is not used in the normal way, but instead just deleted by CMS_ContentInfo_free some memory was lost. Fixes #21985 Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22044) --- diff --git a/crypto/cms/cms_asn1.c b/crypto/cms/cms_asn1.c index ebbc8e1bcde..d393cbcfe38 100644 --- a/crypto/cms/cms_asn1.c +++ b/crypto/cms/cms_asn1.c @@ -51,6 +51,7 @@ static int cms_si_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, EVP_PKEY_free(si->pkey); X509_free(si->signer); EVP_MD_CTX_free(si->mctx); + EVP_PKEY_CTX_free(si->pctx); } return 1; } @@ -89,11 +90,21 @@ ASN1_SEQUENCE(CMS_OriginatorInfo) = { ASN1_IMP_SET_OF_OPT(CMS_OriginatorInfo, crls, CMS_RevocationInfoChoice, 1) } static_ASN1_SEQUENCE_END(CMS_OriginatorInfo) -ASN1_NDEF_SEQUENCE(CMS_EncryptedContentInfo) = { +static int cms_ec_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, + void *exarg) +{ + CMS_EncryptedContentInfo *ec = (CMS_EncryptedContentInfo *)*pval; + + if (operation == ASN1_OP_FREE_POST) + OPENSSL_clear_free(ec->key, ec->keylen); + return 1; +} + +ASN1_NDEF_SEQUENCE_cb(CMS_EncryptedContentInfo, cms_ec_cb) = { ASN1_SIMPLE(CMS_EncryptedContentInfo, contentType, ASN1_OBJECT), ASN1_SIMPLE(CMS_EncryptedContentInfo, contentEncryptionAlgorithm, X509_ALGOR), ASN1_IMP_OPT(CMS_EncryptedContentInfo, encryptedContent, ASN1_OCTET_STRING_NDEF, 0) -} static_ASN1_NDEF_SEQUENCE_END(CMS_EncryptedContentInfo) +} ASN1_NDEF_SEQUENCE_END_cb(CMS_EncryptedContentInfo, CMS_EncryptedContentInfo) ASN1_SEQUENCE(CMS_KeyTransRecipientInfo) = { ASN1_EMBED(CMS_KeyTransRecipientInfo, version, INT32), @@ -317,6 +328,10 @@ static int cms_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, return 0; break; + case ASN1_OP_FREE_POST: + OPENSSL_free(cms->ctx.propq); + break; + } return 1; } diff --git a/crypto/cms/cms_env.c b/crypto/cms/cms_env.c index 99cf1dcb396..445a16fb771 100644 --- a/crypto/cms/cms_env.c +++ b/crypto/cms/cms_env.c @@ -51,15 +51,6 @@ static int cms_get_enveloped_type(const CMS_ContentInfo *cms) return ret; } -void ossl_cms_env_enc_content_free(const CMS_ContentInfo *cinf) -{ - if (cms_get_enveloped_type_simple(cinf) != 0) { - CMS_EncryptedContentInfo *ec = ossl_cms_get0_env_enc_content(cinf); - if (ec != NULL) - OPENSSL_clear_free(ec->key, ec->keylen); - } -} - CMS_EnvelopedData *ossl_cms_get0_enveloped(CMS_ContentInfo *cms) { if (OBJ_obj2nid(cms->contentType) != NID_pkcs7_enveloped) { diff --git a/crypto/cms/cms_lib.c b/crypto/cms/cms_lib.c index 4cc7e3cb61d..6e66f0ecf22 100644 --- a/crypto/cms/cms_lib.c +++ b/crypto/cms/cms_lib.c @@ -21,6 +21,7 @@ static STACK_OF(CMS_CertificateChoices) **cms_get0_certificate_choices(CMS_ContentInfo *cms); +IMPLEMENT_ASN1_ALLOC_FUNCTIONS(CMS_ContentInfo) IMPLEMENT_ASN1_PRINT_FUNCTION(CMS_ContentInfo) CMS_ContentInfo *d2i_CMS_ContentInfo(CMS_ContentInfo **a, @@ -67,20 +68,6 @@ CMS_ContentInfo *CMS_ContentInfo_new_ex(OSSL_LIB_CTX *libctx, const char *propq) return ci; } -CMS_ContentInfo *CMS_ContentInfo_new(void) -{ - return CMS_ContentInfo_new_ex(NULL, NULL); -} - -void CMS_ContentInfo_free(CMS_ContentInfo *cms) -{ - if (cms != NULL) { - ossl_cms_env_enc_content_free(cms); - OPENSSL_free(cms->ctx.propq); - ASN1_item_free((ASN1_VALUE *)cms, ASN1_ITEM_rptr(CMS_ContentInfo)); - } -} - const CMS_CTX *ossl_cms_get0_cmsctx(const CMS_ContentInfo *cms) { return cms != NULL ? &cms->ctx : NULL; diff --git a/crypto/cms/cms_local.h b/crypto/cms/cms_local.h index c262a906f81..0434cb79198 100644 --- a/crypto/cms/cms_local.h +++ b/crypto/cms/cms_local.h @@ -368,6 +368,7 @@ struct CMS_Receipt_st { DECLARE_ASN1_FUNCTIONS(CMS_ContentInfo) DECLARE_ASN1_ITEM(CMS_SignerInfo) +DECLARE_ASN1_ITEM(CMS_EncryptedContentInfo) DECLARE_ASN1_ITEM(CMS_IssuerAndSerialNumber) DECLARE_ASN1_ITEM(CMS_Attributes_Sign) DECLARE_ASN1_ITEM(CMS_Attributes_Verify) @@ -444,7 +445,6 @@ BIO *ossl_cms_EnvelopedData_init_bio(CMS_ContentInfo *cms); int ossl_cms_EnvelopedData_final(CMS_ContentInfo *cms, BIO *chain); BIO *ossl_cms_AuthEnvelopedData_init_bio(CMS_ContentInfo *cms); int ossl_cms_AuthEnvelopedData_final(CMS_ContentInfo *cms, BIO *cmsbio); -void ossl_cms_env_enc_content_free(const CMS_ContentInfo *cinf); CMS_EnvelopedData *ossl_cms_get0_enveloped(CMS_ContentInfo *cms); CMS_AuthEnvelopedData *ossl_cms_get0_auth_enveloped(CMS_ContentInfo *cms); CMS_EncryptedContentInfo *ossl_cms_get0_env_enc_content(const CMS_ContentInfo *cms); diff --git a/crypto/cms/cms_sd.c b/crypto/cms/cms_sd.c index 9788322f8d4..4b7b153d3b9 100644 --- a/crypto/cms/cms_sd.c +++ b/crypto/cms/cms_sd.c @@ -513,8 +513,12 @@ CMS_SignerInfo *CMS_add1_signer(CMS_ContentInfo *cms, ossl_cms_ctx_get0_libctx(ctx), ossl_cms_ctx_get0_propq(ctx), pk, NULL) <= 0) { + si->pctx = NULL; goto err; } + else { + EVP_MD_CTX_set_flags(si->mctx, EVP_MD_CTX_FLAG_KEEP_PKEY_CTX); + } } if (!sd->signerInfos) @@ -756,6 +760,7 @@ static int cms_SignerInfo_content_sign(CMS_ContentInfo *cms, unsigned int mdlen; pctx = si->pctx; + si->pctx = NULL; if (!EVP_DigestFinal_ex(mctx, md, &mdlen)) goto err; siglen = EVP_PKEY_get_size(si->pkey); @@ -844,6 +849,7 @@ int CMS_SignerInfo_sign(CMS_SignerInfo *si) ossl_cms_ctx_get0_propq(ctx), si->pkey, NULL) <= 0) goto err; + EVP_MD_CTX_set_flags(mctx, EVP_MD_CTX_FLAG_KEEP_PKEY_CTX); si->pctx = pctx; } @@ -915,9 +921,16 @@ int CMS_SignerInfo_verify(CMS_SignerInfo *si) goto err; } mctx = si->mctx; + if (si->pctx != NULL) { + EVP_PKEY_CTX_free(si->pctx); + si->pctx = NULL; + } if (EVP_DigestVerifyInit_ex(mctx, &si->pctx, EVP_MD_get0_name(md), libctx, - propq, si->pkey, NULL) <= 0) + propq, si->pkey, NULL) <= 0) { + si->pctx = NULL; goto err; + } + EVP_MD_CTX_set_flags(mctx, EVP_MD_CTX_FLAG_KEEP_PKEY_CTX); if (!cms_sd_asn1_ctrl(si, 1)) goto err; @@ -1034,8 +1047,11 @@ int CMS_SignerInfo_verify_content(CMS_SignerInfo *si, BIO *chain) if (EVP_PKEY_CTX_set_signature_md(pkctx, md) <= 0) goto err; si->pctx = pkctx; - if (!cms_sd_asn1_ctrl(si, 1)) + if (!cms_sd_asn1_ctrl(si, 1)) { + si->pctx = NULL; goto err; + } + si->pctx = NULL; r = EVP_PKEY_verify(pkctx, si->signature->data, si->signature->length, mval, mlen); if (r <= 0) { diff --git a/crypto/cms/cms_smime.c b/crypto/cms/cms_smime.c index d7719267c8c..65da2452fce 100644 --- a/crypto/cms/cms_smime.c +++ b/crypto/cms/cms_smime.c @@ -236,7 +236,7 @@ CMS_ContentInfo *CMS_EncryptedData_encrypt_ex(BIO *in, const EVP_CIPHER *cipher, if (cms == NULL) return NULL; if (!CMS_EncryptedData_set1_key(cms, cipher, key, keylen)) - return NULL; + goto err; if (!(flags & CMS_DETACHED)) CMS_set_detached(cms, 0); @@ -245,6 +245,7 @@ CMS_ContentInfo *CMS_EncryptedData_encrypt_ex(BIO *in, const EVP_CIPHER *cipher, || CMS_final(cms, in, NULL, flags)) return cms; + err: CMS_ContentInfo_free(cms); return NULL; } diff --git a/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d08309 b/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d08309 new file mode 100644 index 00000000000..8b18bb15d5f --- /dev/null +++ b/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d08309 @@ -0,0 +1 @@ +0  *†H†÷  \ No newline at end of file diff --git a/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed b/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed new file mode 100644 index 00000000000..0aabaa3fdfc --- /dev/null +++ b/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed @@ -0,0 +1 @@ +0  *†H†÷  \ No newline at end of file diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t index ea378261105..0848bd1e898 100644 --- a/test/recipes/80-test_cms.t +++ b/test/recipes/80-test_cms.t @@ -399,6 +399,13 @@ my @smime_cms_tests = ( "-out", "{output}.txt" ], \&final_compare ], + + [ "encrypted content test streaming PEM format -noout, 128 bit AES key", + [ "{cmd1}", @prov, "-EncryptedData_encrypt", "-in", $smcont, "-outform", "PEM", + "-aes128", "-secretkey", "000102030405060708090A0B0C0D0E0F", + "-stream", "-noout" ], + [ "{cmd2}", @prov, "-help" ] + ], ); my @smime_cms_cades_tests = (