Fix memory leaks and other mistakes on errors
authorAlessandro Ghedini <alessandro@ghedini.me>
Thu, 8 Oct 2015 12:38:57 +0000 (14:38 +0200)
committerRichard Levitte <levitte@openssl.org>
Fri, 23 Oct 2015 18:38:52 +0000 (20:38 +0200)
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 3f6c7691870d1cd2ad0e0c83638cef3f35a0b548)

crypto/bn/bn_gf2m.c
crypto/bn/bn_recp.c
crypto/bn/bn_x931p.c
crypto/dsa/dsa_gen.c
crypto/evp/evp_key.c
crypto/hmac/hm_ameth.c
crypto/pem/pvkfmt.c
crypto/pkcs12/p12_add.c
ssl/s3_clnt.c

index a0ba8de31ad4e52199f2db8a20fa6ac0802f8acb..a7a04f8ad9e96e47f8d9b852a3b41fe845df121d 100644 (file)
@@ -576,7 +576,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;
 
index 6826f93b3882f526cb584d3109a20b6d8b57e104..7497ac624d94e5d411f0e5ec7e980ef5a907d689 100644 (file)
@@ -152,8 +152,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);
     }
index 6d76b1284e107d8dda985c65d11d7715f7df1ae1..efa48bdf87724abd79a4ca7c6733cf9473b56e6a 100644 (file)
@@ -213,14 +213,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))
@@ -234,6 +234,9 @@ int BN_X931_generate_Xpq(BIGNUM *Xp, BIGNUM *Xq, int nbits, BN_CTX *ctx)
 
     return 0;
 
+ err:
+    BN_CTX_end(ctx);
+    return 0;
 }
 
 /*
index 0dada3d25e974cc4a907eea904e4290815380ad1..59ad0fdab9e6888fb8e430919f4749de5338c154 100644 (file)
@@ -174,14 +174,14 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, size_t qbits,
     if (seed_in != NULL)
         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);
index 71fa627b20d3ed22e3c238cda5640e09e05c26d1..122bc28df530647f01d66e324125ff385332bc51 100644 (file)
@@ -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;
 }
index 641c797ef1d5c02ad011662f3ee34be769db90f4..cf147437eaf0a30ffedca07b8bbcd7372c81fb94 100644 (file)
@@ -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)
index ee4b6a8241cc2c37e6ed7b5cf4a674937eff6817..54df0b44b16ae6de7a28f86c692b23a89ed6b3d0 100644 (file)
@@ -692,23 +692,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;
index 982805d988de28fe32277bb74b7f8e330ae08648..d9f03a39fd1532af0972904d231810231e70a938 100644 (file)
@@ -75,15 +75,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())) {
         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 */
@@ -127,6 +131,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;
     }
 
@@ -144,14 +149,18 @@ PKCS7 *PKCS12_pack_p7data(STACK_OF(PKCS12_SAFEBAG) *sk)
     p7->type = OBJ_nid2obj(NID_pkcs7_data);
     if (!(p7->d.data = M_ASN1_OCTET_STRING_new())) {
         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 */
@@ -181,7 +190,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);
@@ -193,7 +202,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;
@@ -202,10 +211,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,
index 47b3189a786787d17fc7d14eee15f00c578436ee..e5f0fee0578cbcf07b9acb31324b5d3e1c66d4f6 100644 (file)
@@ -2377,6 +2377,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;