Fix a possible memory leak in do_othername

Since the gen->type will not be set in a2i_GENERAL_NAME
the gen->d.otherName will not be automatically
cleaned up by GENERAL_NAME_free.
Also fixed a similar leak in a2i_GENERAL_NAME,
where ASN1_STRING_set may fail but gen->d.ia5
will not be automatically cleaned up.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22996)

diff --git a/crypto/x509/v3_san.c b/crypto/x509/v3_san.c
index 7798505eec6e8f1ce0ce8776d2cb5b6923cb2968..9adf4947071c8ca0d5a1b2081cb2b897aa782174 100644 (file)
--- a/crypto/x509/v3_san.c
+++ b/crypto/x509/v3_san.c
@@ -581,6 +581,8 @@ GENERAL_NAME *a2i_GENERAL_NAME(GENERAL_NAME *out,
         if ((gen->d.ia5 = ASN1_IA5STRING_new()) == NULL ||
             !ASN1_STRING_set(gen->d.ia5, (unsigned char *)value,
                              strlen(value))) {
+            ASN1_IA5STRING_free(gen->d.ia5);
+            gen->d.ia5 = NULL;
             ERR_raise(ERR_LIB_X509V3, ERR_R_ASN1_LIB);
             goto err;
         }
@@ -651,16 +653,21 @@ static int do_othername(GENERAL_NAME *gen, const char *value, X509V3_CTX *ctx)
      */
     ASN1_TYPE_free(gen->d.otherName->value);
     if ((gen->d.otherName->value = ASN1_generate_v3(p + 1, ctx)) == NULL)
-        return 0;
+        goto err;
     objlen = p - value;
     objtmp = OPENSSL_strndup(value, objlen);
     if (objtmp == NULL)
-        return 0;
+        goto err;
     gen->d.otherName->type_id = OBJ_txt2obj(objtmp, 0);
     OPENSSL_free(objtmp);
     if (!gen->d.otherName->type_id)
-        return 0;
+        goto err;
     return 1;
+
+ err:
+    OTHERNAME_free(gen->d.otherName);
+    gen->d.otherName = NULL;
+    return 0;
 }
 
 static int do_dirname(GENERAL_NAME *gen, const char *value, X509V3_CTX *ctx)

diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t
index 32dc4ded8c89961525b916bada32f2db7f8d47c9..20e338b46f4f507480474606964a2527c5aba15a 100644 (file)
--- a/test/recipes/25-test_req.t
+++ b/test/recipes/25-test_req.t
@@ -15,7 +15,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
 setup("test_req");
 
-plan tests => 104;
+plan tests => 106;
 
 require_ok(srctop_file('test', 'recipes', 'tconversion.pl'));
 
@@ -40,10 +40,14 @@ my @addext_args = ( "openssl", "req", "-new", "-out", "testreq.pem",
                     "-key",  srctop_file(@certs, "ee-key.pem"),
     "-config", srctop_file("test", "test.cnf"), @req_new );
 my $val = "subjectAltName=DNS:example.com";
+my $val1 = "subjectAltName=otherName:1.2.3.4;UTF8:test,email:info\@example.com";
 my $val2 = " " . $val;
 my $val3 = $val;
 $val3 =~ s/=/    =/;
 ok( run(app([@addext_args, "-addext", $val])));
+ok( run(app([@addext_args, "-addext", $val1])));
+$val1 =~ s/UTF8/XXXX/; # execute the error handling in do_othername
+ok(!run(app([@addext_args, "-addext", $val1])));
 ok(!run(app([@addext_args, "-addext", $val, "-addext", $val])));
 ok(!run(app([@addext_args, "-addext", $val, "-addext", $val2])));
 ok(!run(app([@addext_args, "-addext", $val, "-addext", $val3])));