PKCS#7: Fix NULL dereference with missing EncryptedContent.

[openssl.git] / crypto / pkcs7 / pk7_doit.c

diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c
index 51e9c6e80a1741470a92663ffd3d4cdd1e51cfb5..cc2f3be5543e3d5395efae6bb28115beb3061e72 100644 (file)
--- a/crypto/pkcs7/pk7_doit.c
+++ b/crypto/pkcs7/pk7_doit.c
@@ -57,7 +57,7 @@
  */
 
 #include <stdio.h>
-#include "cryptlib.h"
+#include "internal/cryptlib.h"
 #include <openssl/rand.h>
 #include <openssl/objects.h>
 #include <openssl/x509.h>
@@ -181,8 +181,7 @@ static int pkcs7_encode_rinfo(PKCS7_RECIP_INFO *ri,
  err:
     EVP_PKEY_free(pkey);
     EVP_PKEY_CTX_free(pctx);
-    if (ek)
-        OPENSSL_free(ek);
+    OPENSSL_free(ek);
     return ret;
 
 }
@@ -229,17 +228,13 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
 
     ret = 1;
 
-    if (*pek) {
-        OPENSSL_cleanse(*pek, *peklen);
-        OPENSSL_free(*pek);
-    }
-
+    OPENSSL_clear_free(*pek, *peklen);
     *pek = ek;
     *peklen = eklen;
 
  err:
     EVP_PKEY_CTX_free(pctx);
-    if (!ret && ek)
+    if (!ret)
         OPENSSL_free(ek);
 
     return ret;
@@ -437,6 +432,12 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
 
     switch (i) {
     case NID_pkcs7_signed:
+        /*
+         * p7->d.sign->contents is a PKCS7 structure consisting of a contentType
+         * field and optional content.
+         * data_body is NULL if that structure has no (=detached) content
+         * or if the contentType is wrong (i.e., not "data").
+         */
         data_body = PKCS7_get_octet_string(p7->d.sign->contents);
         if (!PKCS7_is_detached(p7) && data_body == NULL) {
             PKCS7err(PKCS7_F_PKCS7_DATADECODE,
@@ -448,6 +449,7 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
     case NID_pkcs7_signedAndEnveloped:
         rsk = p7->d.signed_and_enveloped->recipientinfo;
         md_sk = p7->d.signed_and_enveloped->md_algs;
+        /* data_body is NULL if the optional EncryptedContent is missing. */
         data_body = p7->d.signed_and_enveloped->enc_data->enc_data;
         enc_alg = p7->d.signed_and_enveloped->enc_data->algorithm;
         evp_cipher = EVP_get_cipherbyobj(enc_alg->algorithm);
@@ -460,6 +462,7 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
     case NID_pkcs7_enveloped:
         rsk = p7->d.enveloped->recipientinfo;
         enc_alg = p7->d.enveloped->enc_data->algorithm;
+        /* data_body is NULL if the optional EncryptedContent is missing. */
         data_body = p7->d.enveloped->enc_data->enc_data;
         evp_cipher = EVP_get_cipherbyobj(enc_alg->algorithm);
         if (evp_cipher == NULL) {
@@ -473,6 +476,12 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
         goto err;
     }
 
+    /* Detached content must be supplied via in_bio instead. */
+    if (data_body == NULL && in_bio == NULL) {
+        PKCS7err(PKCS7_F_PKCS7_DATADECODE, PKCS7_R_NO_CONTENT);
+        goto err;
+    }
+
     /* We will be checking the signature */
     if (md_sk != NULL) {
         for (i = 0; i < sk_X509_ALGOR_num(md_sk); i++) {
@@ -576,8 +585,7 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
              */
             if (!EVP_CIPHER_CTX_set_key_length(evp_ctx, eklen)) {
                 /* Use random key as MMA defence */
-                OPENSSL_cleanse(ek, eklen);
-                OPENSSL_free(ek);
+                OPENSSL_clear_free(ek, eklen);
                 ek = tkey;
                 eklen = tkeylen;
                 tkey = NULL;
@@ -588,16 +596,10 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
         if (EVP_CipherInit_ex(evp_ctx, NULL, NULL, ek, NULL, 0) <= 0)
             goto err;
 
-        if (ek) {
-            OPENSSL_cleanse(ek, eklen);
-            OPENSSL_free(ek);
-            ek = NULL;
-        }
-        if (tkey) {
-            OPENSSL_cleanse(tkey, tkeylen);
-            OPENSSL_free(tkey);
-            tkey = NULL;
-        }
+        OPENSSL_clear_free(ek, eklen);
+        ek = NULL;
+        OPENSSL_clear_free(tkey, tkeylen);
+        tkey = NULL;
 
         if (out == NULL)
             out = etmp;
@@ -605,7 +607,7 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
             BIO_push(out, etmp);
         etmp = NULL;
     }
-    if (PKCS7_is_detached(p7) || (in_bio != NULL)) {
+    if (in_bio != NULL) {
         bio = in_bio;
     } else {
         if (data_body->length > 0)
@@ -619,23 +621,16 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert)
     }
     BIO_push(out, bio);
     bio = NULL;
-    if (0) {
+    return out;
+
  err:
-        if (ek) {
-            OPENSSL_cleanse(ek, eklen);
-            OPENSSL_free(ek);
-        }
-        if (tkey) {
-            OPENSSL_cleanse(tkey, tkeylen);
-            OPENSSL_free(tkey);
-        }
-        BIO_free_all(out);
-        BIO_free_all(btmp);
-        BIO_free_all(etmp);
-        BIO_free_all(bio);
-        out = NULL;
-    }
-    return (out);
+    OPENSSL_clear_free(ek, eklen);
+    OPENSSL_clear_free(tkey, tkeylen);
+    BIO_free_all(out);
+    BIO_free_all(btmp);
+    BIO_free_all(etmp);
+    BIO_free_all(bio);
+    return  NULL;
 }
 
 static BIO *PKCS7_find_digest(EVP_MD_CTX **pmd, BIO *bio, int nid)
@@ -908,8 +903,7 @@ int PKCS7_SIGNER_INFO_sign(PKCS7_SIGNER_INFO *si)
     return 1;
 
  err:
-    if (abuf)
-        OPENSSL_free(abuf);
+    OPENSSL_free(abuf);
     EVP_MD_CTX_cleanup(&mctx);
     return 0;
 
@@ -1124,7 +1118,7 @@ static ASN1_TYPE *get_attribute(STACK_OF(X509_ATTRIBUTE) *sk, int nid)
 ASN1_OCTET_STRING *PKCS7_digest_from_attributes(STACK_OF(X509_ATTRIBUTE) *sk)
 {
     ASN1_TYPE *astype;
-    if (!(astype = get_attribute(sk, NID_pkcs9_messageDigest)))
+    if ((astype = get_attribute(sk, NID_pkcs9_messageDigest)) == NULL)
         return NULL;
     return astype->value.octet_string;
 }
@@ -1185,11 +1179,10 @@ static int add_attribute(STACK_OF(X509_ATTRIBUTE) **sk, int nid, int atrtype,
     X509_ATTRIBUTE *attr = NULL;
 
     if (*sk == NULL) {
-        *sk = sk_X509_ATTRIBUTE_new_null();
-        if (*sk == NULL)
+        if ((*sk = sk_X509_ATTRIBUTE_new_null()) == NULL)
             return 0;
  new_attrib:
-        if (!(attr = X509_ATTRIBUTE_create(nid, atrtype, value)))
+        if ((attr = X509_ATTRIBUTE_create(nid, atrtype, value)) == NULL)
             return 0;
         if (!sk_X509_ATTRIBUTE_push(*sk, attr)) {
             X509_ATTRIBUTE_free(attr);