From 923df53e256172cd2384e8ec9d7d8079c90a0938 Mon Sep 17 00:00:00 2001 From: Nils Larsch Date: Sat, 3 Feb 2007 09:51:59 +0000 Subject: [PATCH] fix potential memory leaks PR: 1462 Submitted by: Charles Hardin --- crypto/pkcs7/pk7_doit.c | 71 ++++++++++++++++++++++++++------- crypto/pkcs7/pk7_lib.c | 42 ++++++++++++++++---- crypto/pkcs7/pk7_smime.c | 84 +++++++++++++++++++++++++--------------- 3 files changed, 143 insertions(+), 54 deletions(-) diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c index a4bbba0556..a03d7ebedf 100644 --- a/crypto/pkcs7/pk7_doit.c +++ b/crypto/pkcs7/pk7_doit.c @@ -217,7 +217,9 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) keylen=EVP_CIPHER_key_length(evp_cipher); ivlen=EVP_CIPHER_iv_length(evp_cipher); xalg->algorithm = OBJ_nid2obj(EVP_CIPHER_type(evp_cipher)); - if (ivlen > 0) RAND_pseudo_bytes(iv,ivlen); + if (ivlen > 0) + if (RAND_pseudo_bytes(iv,ivlen) <= 0) + goto err; if (EVP_CipherInit_ex(ctx, evp_cipher, NULL, NULL, NULL, 1)<=0) goto err; if (EVP_CIPHER_CTX_rand_key(ctx, key) <= 0) @@ -226,10 +228,13 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) goto err; if (ivlen > 0) { - if (xalg->parameter == NULL) - xalg->parameter=ASN1_TYPE_new(); + if (xalg->parameter == NULL) { + xalg->parameter = ASN1_TYPE_new(); + if (xalg->parameter == NULL) + goto err; + } if(EVP_CIPHER_param_to_asn1(ctx, xalg->parameter) < 0) - goto err; + goto err; } /* Lets do the pub key stuff :-) */ @@ -242,7 +247,8 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) PKCS7err(PKCS7_F_PKCS7_DATAINIT,PKCS7_R_MISSING_CERIPEND_INFO); goto err; } - pkey=X509_get_pubkey(ri->cert); + if ((pkey=X509_get_pubkey(ri->cert)) == NULL) + goto err; jj=EVP_PKEY_size(pkey); EVP_PKEY_free(pkey); if (max < jj) max=jj; @@ -255,7 +261,8 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) for (i=0; icert); + if ((pkey=X509_get_pubkey(ri->cert)) == NULL) + goto err; jj=EVP_PKEY_encrypt(tmp,key,keylen,pkey); EVP_PKEY_free(pkey); if (jj <= 0) @@ -291,6 +298,8 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) if(bio == NULL) { bio=BIO_new(BIO_s_mem()); + if (bio == NULL) + goto err; BIO_set_mem_eof_return(bio,0); } } @@ -541,6 +550,8 @@ BIO *PKCS7_dataDecode(PKCS7 *p7, EVP_PKEY *pkey, BIO *in_bio, X509 *pcert) bio=BIO_new(BIO_s_mem()); BIO_set_mem_eof_return(bio,0); } + if (bio == NULL) + goto err; #endif } BIO_push(out,bio); @@ -695,9 +706,13 @@ int PKCS7_dataFinal(PKCS7 *p7, BIO *bio) ERR_R_MALLOC_FAILURE); goto err; } - PKCS7_add_signed_attribute(si, + if (!PKCS7_add_signed_attribute(si, NID_pkcs9_signingTime, - V_ASN1_UTCTIME,sign_time); + V_ASN1_UTCTIME,sign_time)) + { + M_ASN1_UTCTIME_free(sign_time); + goto err; + } } /* Add digest */ @@ -714,11 +729,16 @@ int PKCS7_dataFinal(PKCS7 *p7, BIO *bio) { PKCS7err(PKCS7_F_PKCS7_DATAFINAL, ERR_R_MALLOC_FAILURE); + M_ASN1_OCTET_STRING_free(digest); goto err; } - PKCS7_add_signed_attribute(si, + if (!PKCS7_add_signed_attribute(si, NID_pkcs9_messageDigest, - V_ASN1_OCTET_STRING,digest); + V_ASN1_OCTET_STRING,digest)) + { + M_ASN1_OCTET_STRING_free(digest); + goto err; + } /* Now sign the attributes */ EVP_SignInit_ex(&ctx_tmp,md_tmp,NULL); @@ -976,8 +996,13 @@ PKCS7_ISSUER_AND_SERIAL *PKCS7_get_issuer_and_serial(PKCS7 *p7, int idx) int i; i=OBJ_obj2nid(p7->type); - if (i != NID_pkcs7_signedAndEnveloped) return(NULL); + if (i != NID_pkcs7_signedAndEnveloped) + return NULL; + if (p7->d.signed_and_enveloped == NULL) + return NULL; rsk=p7->d.signed_and_enveloped->recipientinfo; + if (rsk == NULL) + return NULL; ri=sk_PKCS7_RECIP_INFO_value(rsk,0); if (sk_PKCS7_RECIP_INFO_num(rsk) <= idx) return(NULL); ri=sk_PKCS7_RECIP_INFO_value(rsk,idx); @@ -1031,6 +1056,8 @@ int PKCS7_set_signed_attributes(PKCS7_SIGNER_INFO *p7si, if (p7si->auth_attr != NULL) sk_X509_ATTRIBUTE_pop_free(p7si->auth_attr,X509_ATTRIBUTE_free); p7si->auth_attr=sk_X509_ATTRIBUTE_dup(sk); + if (p7si->auth_attr == NULL) + return 0; for (i=0; iauth_attr,i, @@ -1049,6 +1076,8 @@ int PKCS7_set_attributes(PKCS7_SIGNER_INFO *p7si, STACK_OF(X509_ATTRIBUTE) *sk) sk_X509_ATTRIBUTE_pop_free(p7si->unauth_attr, X509_ATTRIBUTE_free); p7si->unauth_attr=sk_X509_ATTRIBUTE_dup(sk); + if (p7si->unauth_attr == NULL) + return 0; for (i=0; iunauth_attr,i, @@ -1078,10 +1107,16 @@ static int add_attribute(STACK_OF(X509_ATTRIBUTE) **sk, int nid, int atrtype, if (*sk == NULL) { - *sk = sk_X509_ATTRIBUTE_new_null(); + if (!(*sk = sk_X509_ATTRIBUTE_new_null())) + return 0; new_attrib: - attr=X509_ATTRIBUTE_create(nid,atrtype,value); - sk_X509_ATTRIBUTE_push(*sk,attr); + if (!(attr=X509_ATTRIBUTE_create(nid,atrtype,value))) + return 0; + if (!sk_X509_ATTRIBUTE_push(*sk,attr)) + { + X509_ATTRIBUTE_free(attr); + return 0; + } } else { @@ -1094,7 +1129,13 @@ new_attrib: { X509_ATTRIBUTE_free(attr); attr=X509_ATTRIBUTE_create(nid,atrtype,value); - sk_X509_ATTRIBUTE_set(*sk,i,attr); + if (attr == NULL) + return 0; + if (!sk_X509_ATTRIBUTE_set(*sk,i,attr)) + { + X509_ATTRIBUTE_free(attr); + return 0; + } goto end; } } diff --git a/crypto/pkcs7/pk7_lib.c b/crypto/pkcs7/pk7_lib.c index 58ce6791c9..f2490941a3 100644 --- a/crypto/pkcs7/pk7_lib.c +++ b/crypto/pkcs7/pk7_lib.c @@ -271,16 +271,23 @@ int PKCS7_add_signer(PKCS7 *p7, PKCS7_SIGNER_INFO *psi) if (!j) /* we need to add another algorithm */ { if(!(alg=X509_ALGOR_new()) - || !(alg->parameter = ASN1_TYPE_new())) { + || !(alg->parameter = ASN1_TYPE_new())) + { + X509_ALGOR_free(alg); PKCS7err(PKCS7_F_PKCS7_ADD_SIGNER,ERR_R_MALLOC_FAILURE); return(0); - } + } alg->algorithm=OBJ_nid2obj(nid); alg->parameter->type = V_ASN1_NULL; - sk_X509_ALGOR_push(md_sk,alg); + if (!sk_X509_ALGOR_push(md_sk,alg)) + { + X509_ALGOR_free(alg); + return 0; + } } - sk_PKCS7_SIGNER_INFO_push(signer_sk,psi); + if (!sk_PKCS7_SIGNER_INFO_push(signer_sk,psi)) + return 0; return(1); } @@ -305,8 +312,17 @@ int PKCS7_add_certificate(PKCS7 *p7, X509 *x509) if (*sk == NULL) *sk=sk_X509_new_null(); + if (*sk == NULL) + { + PKCS7err(PKCS7_F_PKCS7_ADD_CERTIFICATE,ERR_R_MALLOC_FAILURE); + return 0; + } CRYPTO_add(&x509->references,1,CRYPTO_LOCK_X509); - sk_X509_push(*sk,x509); + if (!sk_X509_push(*sk,x509)) + { + X509_free(x509); + return 0; + } return(1); } @@ -331,9 +347,18 @@ int PKCS7_add_crl(PKCS7 *p7, X509_CRL *crl) if (*sk == NULL) *sk=sk_X509_CRL_new_null(); + if (*sk == NULL) + { + PKCS7err(PKCS7_F_PKCS7_ADD_CRL,ERR_R_MALLOC_FAILURE); + return 0; + } CRYPTO_add(&crl->references,1,CRYPTO_LOCK_X509_CRL); - sk_X509_CRL_push(*sk,crl); + if (!sk_X509_CRL_push(*sk,crl)) + { + X509_CRL_free(crl); + return 0; + } return(1); } @@ -424,6 +449,7 @@ PKCS7_SIGNER_INFO *PKCS7_add_signature(PKCS7 *p7, X509 *x509, EVP_PKEY *pkey, if (!PKCS7_add_signer(p7,si)) goto err; return(si); err: + PKCS7_SIGNER_INFO_free(si); return(NULL); } @@ -468,6 +494,7 @@ PKCS7_RECIP_INFO *PKCS7_add_recipient(PKCS7 *p7, X509 *x509) if (!PKCS7_add_recipient_info(p7,ri)) goto err; return(ri); err: + PKCS7_RECIP_INFO_free(ri); return(NULL); } @@ -490,7 +517,8 @@ int PKCS7_add_recipient_info(PKCS7 *p7, PKCS7_RECIP_INFO *ri) return(0); } - sk_PKCS7_RECIP_INFO_push(sk,ri); + if (!sk_PKCS7_RECIP_INFO_push(sk,ri)) + return 0; return(1); } diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c index dc835e5b8a..fab85137b7 100644 --- a/crypto/pkcs7/pk7_smime.c +++ b/crypto/pkcs7/pk7_smime.c @@ -66,10 +66,10 @@ PKCS7 *PKCS7_sign(X509 *signcert, EVP_PKEY *pkey, STACK_OF(X509) *certs, BIO *data, int flags) { - PKCS7 *p7; + PKCS7 *p7 = NULL; PKCS7_SIGNER_INFO *si; - BIO *p7bio; - STACK_OF(X509_ALGOR) *smcap; + BIO *p7bio = NULL; + STACK_OF(X509_ALGOR) *smcap = NULL; int i; if(!X509_check_private_key(signcert, pkey)) { @@ -82,48 +82,58 @@ PKCS7 *PKCS7_sign(X509 *signcert, EVP_PKEY *pkey, STACK_OF(X509) *certs, return NULL; } - PKCS7_set_type(p7, NID_pkcs7_signed); + if (!PKCS7_set_type(p7, NID_pkcs7_signed)) + goto err; - PKCS7_content_new(p7, NID_pkcs7_data); + if (!PKCS7_content_new(p7, NID_pkcs7_data)) + goto err; - if (!(si = PKCS7_add_signature(p7,signcert,pkey,EVP_sha1()))) { + if (!(si = PKCS7_add_signature(p7,signcert,pkey,EVP_sha1()))) { PKCS7err(PKCS7_F_PKCS7_SIGN,PKCS7_R_PKCS7_ADD_SIGNATURE_ERROR); - PKCS7_free(p7); - return NULL; + goto err; } if(!(flags & PKCS7_NOCERTS)) { - PKCS7_add_certificate(p7, signcert); + if (!PKCS7_add_certificate(p7, signcert)) + goto err; if(certs) for(i = 0; i < sk_X509_num(certs); i++) - PKCS7_add_certificate(p7, sk_X509_value(certs, i)); + if (!PKCS7_add_certificate(p7, sk_X509_value(certs, i))) + goto err; } if(!(flags & PKCS7_NOATTR)) { - PKCS7_add_signed_attribute(si, NID_pkcs9_contentType, - V_ASN1_OBJECT, OBJ_nid2obj(NID_pkcs7_data)); + if (!PKCS7_add_signed_attribute(si, NID_pkcs9_contentType, + V_ASN1_OBJECT, OBJ_nid2obj(NID_pkcs7_data))) + goto err; /* Add SMIMECapabilities */ if(!(flags & PKCS7_NOSMIMECAP)) { if(!(smcap = sk_X509_ALGOR_new_null())) { PKCS7err(PKCS7_F_PKCS7_SIGN,ERR_R_MALLOC_FAILURE); - PKCS7_free(p7); - return NULL; + goto err; } #ifndef OPENSSL_NO_DES - PKCS7_simple_smimecap (smcap, NID_des_ede3_cbc, -1); + if (!PKCS7_simple_smimecap (smcap, NID_des_ede3_cbc, -1)) + goto err; #endif #ifndef OPENSSL_NO_RC2 - PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 128); - PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 64); + if (!PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 128)) + goto err; + if (!PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 64)) + goto err; #endif #ifndef OPENSSL_NO_DES - PKCS7_simple_smimecap (smcap, NID_des_cbc, -1); + if (!PKCS7_simple_smimecap (smcap, NID_des_cbc, -1)) + goto err; #endif #ifndef OPENSSL_NO_RC2 - PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 40); + if (!PKCS7_simple_smimecap (smcap, NID_rc2_cbc, 40)) + goto err; #endif - PKCS7_add_attrib_smimecap (si, smcap); + if (!PKCS7_add_attrib_smimecap (si, smcap)) + goto err; sk_X509_ALGOR_pop_free(smcap, X509_ALGOR_free); + smcap = NULL; } } @@ -135,22 +145,24 @@ PKCS7 *PKCS7_sign(X509 *signcert, EVP_PKEY *pkey, STACK_OF(X509) *certs, if (!(p7bio = PKCS7_dataInit(p7, NULL))) { PKCS7err(PKCS7_F_PKCS7_SIGN,ERR_R_MALLOC_FAILURE); - PKCS7_free(p7); - return NULL; + goto err; } SMIME_crlf_copy(data, p7bio, flags); - if (!PKCS7_dataFinal(p7,p7bio)) { + if (!PKCS7_dataFinal(p7,p7bio)) { PKCS7err(PKCS7_F_PKCS7_SIGN,PKCS7_R_PKCS7_DATASIGN); - PKCS7_free(p7); - BIO_free_all(p7bio); - return NULL; + goto err; } BIO_free_all(p7bio); return p7; +err: + sk_X509_ALGOR_pop_free(smcap, X509_ALGOR_free); + BIO_free_all(p7bio); + PKCS7_free(p7); + return NULL; } int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, @@ -262,7 +274,8 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store, tmpin = indata; - p7bio=PKCS7_dataInit(p7,tmpin); + if (!(p7bio=PKCS7_dataInit(p7,tmpin))) + goto err; if(flags & PKCS7_TEXT) { if(!(tmpout = BIO_new(BIO_s_mem()))) { @@ -341,7 +354,7 @@ STACK_OF(X509) *PKCS7_get0_signers(PKCS7 *p7, STACK_OF(X509) *certs, int flags) if(sk_PKCS7_SIGNER_INFO_num(sinfos) <= 0) { PKCS7err(PKCS7_F_PKCS7_GET0_SIGNERS,PKCS7_R_NO_SIGNERS); - return 0; + return NULL; } if(!(signers = sk_X509_new_null())) { @@ -364,10 +377,13 @@ STACK_OF(X509) *PKCS7_get0_signers(PKCS7 *p7, STACK_OF(X509) *certs, int flags) if (!signer) { PKCS7err(PKCS7_F_PKCS7_GET0_SIGNERS,PKCS7_R_SIGNER_CERTIFICATE_NOT_FOUND); sk_X509_free(signers); - return 0; + return NULL; } - sk_X509_push(signers, signer); + if (!sk_X509_push(signers, signer)) { + sk_X509_free(signers); + return NULL; + } } return signers; } @@ -387,7 +403,8 @@ PKCS7 *PKCS7_encrypt(STACK_OF(X509) *certs, BIO *in, const EVP_CIPHER *cipher, return NULL; } - PKCS7_set_type(p7, NID_pkcs7_enveloped); + if (!PKCS7_set_type(p7, NID_pkcs7_enveloped)) + goto err; if(!PKCS7_set_cipher(p7, cipher)) { PKCS7err(PKCS7_F_PKCS7_ENCRYPT,PKCS7_R_ERROR_SETTING_CIPHER); goto err; @@ -421,7 +438,7 @@ PKCS7 *PKCS7_encrypt(STACK_OF(X509) *certs, BIO *in, const EVP_CIPHER *cipher, err: - BIO_free(p7bio); + BIO_free_all(p7bio); PKCS7_free(p7); return NULL; @@ -459,10 +476,13 @@ int PKCS7_decrypt(PKCS7 *p7, EVP_PKEY *pkey, X509 *cert, BIO *data, int flags) /* Encrypt BIOs can't do BIO_gets() so add a buffer BIO */ if(!(tmpbuf = BIO_new(BIO_f_buffer()))) { PKCS7err(PKCS7_F_PKCS7_DECRYPT, ERR_R_MALLOC_FAILURE); + BIO_free_all(tmpmem); return 0; } if(!(bread = BIO_push(tmpbuf, tmpmem))) { PKCS7err(PKCS7_F_PKCS7_DECRYPT, ERR_R_MALLOC_FAILURE); + BIO_free_all(tmpbuf); + BIO_free_all(tmpmem); return 0; } ret = SMIME_text(bread, data);