GH354: Memory leak fixes

Fix more potential leaks in X509_verify_cert()
Fix memory leak in ClientHello test
Fix memory leak in gost2814789 test
Fix potential memory leak in PKCS7_verify()
Fix potential memory leaks in X509_add1_reject_object()
Refactor to use "goto err" in cleanup.

Signed-off-by: Rich Salz <rsalz@akamai.com>
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 55500ea7c46c27a150a46832e1260891aaad8e52)

diff --git a/crypto/asn1/x_x509a.c b/crypto/asn1/x_x509a.c
index 76bbc1370ff7fbd704935231cfeff110a4a8c446..ad93592a714a2adfb44ec44044810688117cab44 100644 (file)
--- a/crypto/asn1/x_x509a.c
+++ b/crypto/asn1/x_x509a.c
@@ -163,10 +163,13 @@ int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj)
     if (!(objtmp = OBJ_dup(obj)))
         return 0;
     if (!(aux = aux_get(x)))
-        return 0;
+        goto err;
     if (!aux->reject && !(aux->reject = sk_ASN1_OBJECT_new_null()))
-        return 0;
+        goto err;
     return sk_ASN1_OBJECT_push(aux->reject, objtmp);
+ err:
+    ASN1_OBJECT_free(objtmp);
+    return 0;
 }
 
 void X509_trust_clear(X509 *x)

diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c
index dbd4100c8d028bb9031ebcbc9b67c63ef0cd18d9..c4d3724d2a48b018f08d9bc33c3451853bc14b12 100644 (file)
--- a/crypto/pkcs7/pk7_smime.c
+++ b/crypto/pkcs7/pk7_smime.c
@@ -256,8 +256,8 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
     X509_STORE_CTX cert_ctx;
     char buf[4096];
     int i, j = 0, k, ret = 0;
-    BIO *p7bio;
-    BIO *tmpin, *tmpout;
+    BIO *p7bio = NULL;
+    BIO *tmpin = NULL, *tmpout = NULL;
 
     if (!p7) {
         PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_INVALID_NULL_POINTER);
@@ -274,18 +274,12 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
         PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_NO_CONTENT);
         return 0;
     }
-#if 0
-    /*
-     * NB: this test commented out because some versions of Netscape
-     * illegally include zero length content when signing data.
-     */
 
     /* Check for data and content: two sets of data */
     if (!PKCS7_get_detached(p7) && indata) {
         PKCS7err(PKCS7_F_PKCS7_VERIFY, PKCS7_R_CONTENT_AND_DATA_PRESENT);
         return 0;
     }
-#endif
 
     sinfos = PKCS7_get_signer_info(p7);
 
@@ -295,7 +289,6 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
     }
 
     signers = PKCS7_get0_signers(p7, certs, flags);
-
     if (!signers)
         return 0;
 
@@ -308,14 +301,12 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
                 if (!X509_STORE_CTX_init(&cert_ctx, store, signer,
                                          p7->d.sign->cert)) {
                     PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);
-                    sk_X509_free(signers);
-                    return 0;
+                    goto err;
                 }
                 X509_STORE_CTX_set_default(&cert_ctx, "smime_sign");
             } else if (!X509_STORE_CTX_init(&cert_ctx, store, signer, NULL)) {
                 PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_X509_LIB);
-                sk_X509_free(signers);
-                return 0;
+                goto err;
             }
             if (!(flags & PKCS7_NOCRL))
                 X509_STORE_CTX_set0_crls(&cert_ctx, p7->d.sign->crl);
@@ -328,8 +319,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
                          PKCS7_R_CERTIFICATE_VERIFY_ERROR);
                 ERR_add_error_data(2, "Verify error:",
                                    X509_verify_cert_error_string(j));
-                sk_X509_free(signers);
-                return 0;
+                goto err;
             }
             /* Check for revocation status here */
         }
@@ -348,7 +338,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
         tmpin = BIO_new_mem_buf(ptr, len);
         if (tmpin == NULL) {
             PKCS7err(PKCS7_F_PKCS7_VERIFY, ERR_R_MALLOC_FAILURE);
-            return 0;
+            goto err;
         }
     } else
         tmpin = indata;
@@ -398,15 +388,12 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
     ret = 1;
 
  err:
-
     if (tmpin == indata) {
         if (indata)
             BIO_pop(p7bio);
     }
     BIO_free_all(p7bio);
-
     sk_X509_free(signers);
-
     return ret;
 }
 

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 15a4fb98e7646fdd1f9e9eebc63a96e6815f3bff..7bac197666c489eb8cb1feb1eb9d059d5614fcc9 100644 (file)
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -249,7 +249,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
         if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) {
             ok = ctx->get_issuer(&xtmp, ctx, x);
             if (ok < 0)
-                return ok;
+                goto end;
             /*
              * If successful for now free up cert so it will be picked up
              * again later.
@@ -347,7 +347,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
             ok = ctx->get_issuer(&xtmp, ctx, x);
 
             if (ok < 0)
-                return ok;
+                goto end;
             if (ok == 0)
                 break;
             x = xtmp;

diff --git a/ssl/clienthellotest.c b/ssl/clienthellotest.c
index a00a7ea02d6b9ae43b2a1798fa148c66c467ca12..77517c61b1f3474bc4d99329f893a2b4cf2b81ab 100644 (file)
--- a/ssl/clienthellotest.c
+++ b/ssl/clienthellotest.c
@@ -213,6 +213,7 @@ int main(int argc, char *argv[])
     EVP_cleanup();
     CRYPTO_cleanup_all_ex_data();
     CRYPTO_mem_leaks(err);
+    BIO_free(err);
 
     return testresult?0:1;
 }