]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Fix error handling in CMS_EncryptedData_encrypt
authorBernd Edlinger <bernd.edlinger@hotmail.de>
Thu, 7 Sep 2023 16:05:44 +0000 (18:05 +0200)
committerTomas Mraz <tomas@openssl.org>
Wed, 13 Nov 2024 16:12:23 +0000 (17:12 +0100)
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 <hlandau@devever.net>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22044)

crypto/cms/cms_asn1.c
crypto/cms/cms_env.c
crypto/cms/cms_lib.c
crypto/cms/cms_local.h
crypto/cms/cms_sd.c
crypto/cms/cms_smime.c
fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d08309 [new file with mode: 0644]
fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed [new file with mode: 0644]
test/recipes/80-test_cms.t

index ebbc8e1bcded1e22df5e6d07b476e3f404d37dd9..d393cbcfe3842baa3d91f19e083534b2386ec823 100644 (file)
@@ -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;
 }
index 99cf1dcb396ca53127669d5da1846566d18f5dd3..445a16fb771f27e0d3425b7e592069c1564ce107 100644 (file)
@@ -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) {
index 4cc7e3cb61dea1a916b060193c126f794253bf83..6e66f0ecf22cfcf3b754bd9c92b1dd0a74b290bd 100644 (file)
@@ -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;
index c262a906f8130645873836afd75dfc055ea8f1c9..0434cb7919889265257b81698820bf8143a994b2 100644 (file)
@@ -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);
index 9788322f8d4e3a574bd5bae05f05493e839eb833..4b7b153d3b981e5d24ea691dc6a8e6da5b601eec 100644 (file)
@@ -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) {
index d7719267c8c83df729678afd88da361517b2baeb..65da2452fcea7e6be1cdf16d6e2fdc9e45415ec0 100644 (file)
@@ -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 (file)
index 0000000..8b18bb1
--- /dev/null
@@ -0,0 +1 @@
+0\v\ 6    *\86H\86÷\r\ 1\a\ 3
\ No newline at end of file
diff --git a/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed b/fuzz/corpora/cms/2aba8037213156ea1593054768d4576cb8d083ed
new file mode 100644 (file)
index 0000000..0aabaa3
--- /dev/null
@@ -0,0 +1 @@
+0\v\ 6    *\86H\86÷\r\ 1\a\ 6
\ No newline at end of file
index ea3782611054cd1938965b4f7bf1b7f8b4272c0e..0848bd1e8986d54256833479913bf75d24a68e97 100644 (file)
@@ -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 = (