From 3f6c7691870d1cd2ad0e0c83638cef3f35a0b548 Mon Sep 17 00:00:00 2001 From: Alessandro Ghedini Date: Thu, 8 Oct 2015 14:38:57 +0200 Subject: [PATCH] Fix memory leaks and other mistakes on errors Reviewed-by: Rich Salz Reviewed-by: Richard Levitte --- crypto/bn/bn_gf2m.c | 2 +- crypto/bn/bn_recp.c | 4 +++- crypto/bn/bn_x931p.c | 7 +++++-- crypto/dsa/dsa_gen.c | 6 +++--- crypto/evp/evp_key.c | 4 ++-- crypto/hmac/hm_ameth.c | 9 +++++++-- crypto/pem/pvkfmt.c | 8 ++++---- crypto/pkcs12/p12_add.c | 27 ++++++++++++++++++++------- ssl/s3_clnt.c | 1 + 9 files changed, 46 insertions(+), 22 deletions(-) diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index cd137c3649..3b6c883c04 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -574,7 +574,7 @@ int BN_GF2m_mod_sqr_arr(BIGNUM *r, const BIGNUM *a, const int p[], bn_check_top(a); BN_CTX_start(ctx); if ((s = BN_CTX_get(ctx)) == NULL) - return 0; + goto err; if (!bn_wexpand(s, 2 * a->top)) goto err; diff --git a/crypto/bn/bn_recp.c b/crypto/bn/bn_recp.c index 3dc2166c7d..39eed8b297 100644 --- a/crypto/bn/bn_recp.c +++ b/crypto/bn/bn_recp.c @@ -151,8 +151,10 @@ int BN_div_recp(BIGNUM *dv, BIGNUM *rem, const BIGNUM *m, if (BN_ucmp(m, &(recp->N)) < 0) { BN_zero(d); - if (!BN_copy(r, m)) + if (!BN_copy(r, m)) { + BN_CTX_end(ctx); return 0; + } BN_CTX_end(ctx); return (1); } diff --git a/crypto/bn/bn_x931p.c b/crypto/bn/bn_x931p.c index 15ba41dce3..76ce6f67d2 100644 --- a/crypto/bn/bn_x931p.c +++ b/crypto/bn/bn_x931p.c @@ -214,14 +214,14 @@ int BN_X931_generate_Xpq(BIGNUM *Xp, BIGNUM *Xq, int nbits, BN_CTX *ctx) * exceeded. */ if (!BN_rand(Xp, nbits, 1, 0)) - return 0; + goto err; BN_CTX_start(ctx); t = BN_CTX_get(ctx); for (i = 0; i < 1000; i++) { if (!BN_rand(Xq, nbits, 1, 0)) - return 0; + goto err; /* Check that |Xp - Xq| > 2^(nbits - 100) */ BN_sub(t, Xp, Xq); if (BN_num_bits(t) > (nbits - 100)) @@ -235,6 +235,9 @@ int BN_X931_generate_Xpq(BIGNUM *Xp, BIGNUM *Xq, int nbits, BN_CTX *ctx) return 0; + err: + BN_CTX_end(ctx); + return 0; } /* diff --git a/crypto/dsa/dsa_gen.c b/crypto/dsa/dsa_gen.c index 056e50049d..562d0b58d4 100644 --- a/crypto/dsa/dsa_gen.c +++ b/crypto/dsa/dsa_gen.c @@ -142,14 +142,14 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, size_t qbits, memcpy(seed, seed_in, seed_len); } + if ((mont = BN_MONT_CTX_new()) == NULL) + goto err; + if ((ctx = BN_CTX_new()) == NULL) goto err; BN_CTX_start(ctx); - if ((mont = BN_MONT_CTX_new()) == NULL) - goto err; - r0 = BN_CTX_get(ctx); g = BN_CTX_get(ctx); W = BN_CTX_get(ctx); diff --git a/crypto/evp/evp_key.c b/crypto/evp/evp_key.c index 9c34a03441..5c03a91a94 100644 --- a/crypto/evp/evp_key.c +++ b/crypto/evp/evp_key.c @@ -137,7 +137,7 @@ int EVP_BytesToKey(const EVP_CIPHER *type, const EVP_MD *md, EVP_MD_CTX_init(&c); for (;;) { if (!EVP_DigestInit_ex(&c, md, NULL)) - return 0; + goto err; if (addmd++) if (!EVP_DigestUpdate(&c, &(md_buf[0]), mds)) goto err; @@ -188,6 +188,6 @@ int EVP_BytesToKey(const EVP_CIPHER *type, const EVP_MD *md, rv = type->key_len; err: EVP_MD_CTX_cleanup(&c); - OPENSSL_cleanse(&(md_buf[0]), EVP_MAX_MD_SIZE); + OPENSSL_cleanse(md_buf, sizeof(md_buf)); return rv; } diff --git a/crypto/hmac/hm_ameth.c b/crypto/hmac/hm_ameth.c index cd29c0ccd8..20abe4f087 100644 --- a/crypto/hmac/hm_ameth.c +++ b/crypto/hmac/hm_ameth.c @@ -108,9 +108,14 @@ static int old_hmac_decode(EVP_PKEY *pkey, ASN1_OCTET_STRING *os; os = ASN1_OCTET_STRING_new(); if (!os || !ASN1_OCTET_STRING_set(os, *pder, derlen)) - return 0; - EVP_PKEY_assign(pkey, EVP_PKEY_HMAC, os); + goto err; + if (!EVP_PKEY_assign(pkey, EVP_PKEY_HMAC, os)) + goto err; return 1; + + err: + ASN1_OCTET_STRING_free(os); + return 0; } static int old_hmac_encode(const EVP_PKEY *pkey, unsigned char **pder) diff --git a/crypto/pem/pvkfmt.c b/crypto/pem/pvkfmt.c index c682fc793c..342e2c52d8 100644 --- a/crypto/pem/pvkfmt.c +++ b/crypto/pem/pvkfmt.c @@ -686,23 +686,23 @@ static EVP_PKEY *do_PVK_body(const unsigned char **in, inlen = PEM_def_callback(psbuf, PEM_BUFSIZE, 0, u); if (inlen <= 0) { PEMerr(PEM_F_DO_PVK_BODY, PEM_R_BAD_PASSWORD_READ); - return NULL; + goto err; } enctmp = OPENSSL_malloc(keylen + 8); if (!enctmp) { PEMerr(PEM_F_DO_PVK_BODY, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } if (!derive_pvk_key(keybuf, p, saltlen, (unsigned char *)psbuf, inlen)) - return NULL; + goto err; p += saltlen; /* Copy BLOBHEADER across, decrypt rest */ memcpy(enctmp, p, 8); p += 8; if (keylen < 8) { PEMerr(PEM_F_DO_PVK_BODY, PEM_R_PVK_TOO_SHORT); - return NULL; + goto err; } inlen = keylen - 8; q = enctmp + 8; diff --git a/crypto/pkcs12/p12_add.c b/crypto/pkcs12/p12_add.c index 29abe2e1a4..648b16b210 100644 --- a/crypto/pkcs12/p12_add.c +++ b/crypto/pkcs12/p12_add.c @@ -76,15 +76,19 @@ PKCS12_SAFEBAG *PKCS12_item_pack_safebag(void *obj, const ASN1_ITEM *it, bag->type = OBJ_nid2obj(nid1); if (!ASN1_item_pack(obj, it, &bag->value.octet)) { PKCS12err(PKCS12_F_PKCS12_ITEM_PACK_SAFEBAG, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } if ((safebag = PKCS12_SAFEBAG_new()) == NULL) { PKCS12err(PKCS12_F_PKCS12_ITEM_PACK_SAFEBAG, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } safebag->value.bag = bag; safebag->type = OBJ_nid2obj(nid2); return safebag; + + err: + PKCS12_BAGS_free(bag); + return NULL; } /* Turn PKCS8 object into a keybag */ @@ -129,6 +133,7 @@ PKCS12_SAFEBAG *PKCS12_MAKE_SHKEYBAG(int pbe_nid, const char *pass, PKCS8_encrypt(pbe_nid, pbe_ciph, pass, passlen, salt, saltlen, iter, p8))) { PKCS12err(PKCS12_F_PKCS12_MAKE_SHKEYBAG, ERR_R_MALLOC_FAILURE); + PKCS12_SAFEBAG_free(bag); return NULL; } @@ -147,14 +152,18 @@ PKCS7 *PKCS12_pack_p7data(STACK_OF(PKCS12_SAFEBAG) *sk) p7->type = OBJ_nid2obj(NID_pkcs7_data); if ((p7->d.data = ASN1_OCTET_STRING_new()) == NULL) { PKCS12err(PKCS12_F_PKCS12_PACK_P7DATA, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } if (!ASN1_item_pack(sk, ASN1_ITEM_rptr(PKCS12_SAFEBAGS), &p7->d.data)) { PKCS12err(PKCS12_F_PKCS12_PACK_P7DATA, PKCS12_R_CANT_PACK_STRUCTURE); - return NULL; + goto err; } return p7; + + err: + PKCS7_free(p7); + return NULL; } /* Unpack SAFEBAGS from PKCS#7 data ContentInfo */ @@ -185,7 +194,7 @@ PKCS7 *PKCS12_pack_p7encdata(int pbe_nid, const char *pass, int passlen, if (!PKCS7_set_type(p7, NID_pkcs7_encrypted)) { PKCS12err(PKCS12_F_PKCS12_PACK_P7ENCDATA, PKCS12_R_ERROR_SETTING_ENCRYPTED_DATA_TYPE); - return NULL; + goto err; } pbe_ciph = EVP_get_cipherbynid(pbe_nid); @@ -197,7 +206,7 @@ PKCS7 *PKCS12_pack_p7encdata(int pbe_nid, const char *pass, int passlen, if (!pbe) { PKCS12err(PKCS12_F_PKCS12_PACK_P7ENCDATA, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } X509_ALGOR_free(p7->d.encrypted->enc_data->algorithm); p7->d.encrypted->enc_data->algorithm = pbe; @@ -206,10 +215,14 @@ PKCS7 *PKCS12_pack_p7encdata(int pbe_nid, const char *pass, int passlen, PKCS12_item_i2d_encrypt(pbe, ASN1_ITEM_rptr(PKCS12_SAFEBAGS), pass, passlen, bags, 1))) { PKCS12err(PKCS12_F_PKCS12_PACK_P7ENCDATA, PKCS12_R_ENCRYPT_ERROR); - return NULL; + goto err; } return p7; + + err: + PKCS7_free(p7); + return NULL; } STACK_OF(PKCS12_SAFEBAG) *PKCS12_unpack_p7encdata(PKCS7 *p7, const char *pass, diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 2df5afe14e..c25f801cad 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -2411,6 +2411,7 @@ int ssl3_send_client_key_exchange(SSL *s) || (pkey->pkey.rsa == NULL)) { SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + EVP_PKEY_free(pkey); goto err; } rsa = pkey->pkey.rsa; -- 2.25.1