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 17:52:08 +0000 (19:52 +0200)
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
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 cd137c36496f2322e4e4e4e2df12a72e09356bf9..3b6c883c04b954c3dcb8af784135104c6bb73752 100644 (file)
@@ -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;
 
index 3dc2166c7d91b731271a1992805e541812496567..39eed8b29750d5d06d7772870d1d5fbde98d32a4 100644 (file)
@@ -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);
     }
index 15ba41dce3647640b40f01e1668b1b2070a87349..76ce6f67d27d9689ba8c5048d2a2c6a3c6e49152 100644 (file)
@@ -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;
 }
 
 /*
index 056e50049d0670176a786165477013b6dac2c0f0..562d0b58d4b84248d9659060cf71db1bee948086 100644 (file)
@@ -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);
index 9c34a034418ad3530ecbe8aac38cbb06bdcf2d25..5c03a91a941bcfa4cfe8e323221c78c9cb202b7b 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 cd29c0ccd8ab50afc5becedf61726ce76943abbb..20abe4f0871555a00e6cc50ad5816fefd8ef9127 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 c682fc793c95cfcc0493edf6b96827528ebc6dc8..342e2c52d8205dd3f430f872c12f2b4d090a436e 100644 (file)
@@ -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;
index 29abe2e1a4bd8fc445d691bd80a0c0139c0833d0..648b16b210bea5ff54ef8b550519cc82003e5a1f 100644 (file)
@@ -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,
index 2df5afe14e726ebb2e128798645ec294fca8dbf8..c25f801cadddc4773e79e1854a08fd28c386d029 100644 (file)
@@ -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;