Fix leaks in copy_issuer()

The stack of subject alternative names from the issuer is parsed using
X509V3_EXT_d2i(), so it must be freed with sk_GENERAL_NAME_pop_free().
It's not worth doing complicated ownership handling when the individual
alternative names can be copied with GENERAL_NAME_dup().

Previously, ialt and its remaining members would be leaked when the call
to sk_GENERAL_NAME_push() failed halfway through.

This is only reachable via the issuer:copy x509v3.cnf(5) directive.

ok jsing

diff --git a/lib/libcrypto/x509/x509_alt.c b/lib/libcrypto/x509/x509_alt.c
index c4c5fca..59fa39f 100644 (file)
--- a/lib/libcrypto/x509/x509_alt.c
+++ b/lib/libcrypto/x509/x509_alt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_alt.c,v 1.15 2023/02/16 08:38:17 tb Exp $ */
+/* $OpenBSD: x509_alt.c,v 1.16 2023/08/30 00:49:32 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -354,10 +354,11 @@ err:
 static int
 copy_issuer(X509V3_CTX *ctx, GENERAL_NAMES *gens)
 {
-       GENERAL_NAMES *ialt;
-       GENERAL_NAME *gen;
+       GENERAL_NAMES *ialt = NULL;
+       GENERAL_NAME *gen = NULL;
        X509_EXTENSION *ext;
        int i;
+       int ret = 0;
 
        if (ctx && (ctx->flags == CTX_TEST))
                return 1;
@@ -375,19 +376,24 @@ copy_issuer(X509V3_CTX *ctx, GENERAL_NAMES *gens)
        }
 
        for (i = 0; i < sk_GENERAL_NAME_num(ialt); i++) {
-               gen = sk_GENERAL_NAME_value(ialt, i);
+               GENERAL_NAME *val = sk_GENERAL_NAME_value(ialt, i);
+
+               if ((gen = GENERAL_NAME_dup(val)) == NULL)
+                       goto err;
                if (!sk_GENERAL_NAME_push(gens, gen)) {
                        X509V3error(ERR_R_MALLOC_FAILURE);
                        goto err;
                }
+               gen = NULL;
        }
-       sk_GENERAL_NAME_free(ialt);
 
-       return 1;
+       ret = 1;
 
-err:
-       return 0;
+ err:
+       sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free);
+       GENERAL_NAME_free(gen);
 
+       return ret;
 }
 
 static GENERAL_NAMES *