Skip to content

Commit

Permalink
Fix a possible memory leak in do_othername
Browse files Browse the repository at this point in the history
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 #22996)

(cherry picked from commit 1c07821)
  • Loading branch information
bernd-edlinger authored and t8m committed Dec 12, 2023
1 parent 250990e commit ade7b08
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
13 changes: 10 additions & 3 deletions crypto/x509/v3_san.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion test/recipes/25-test_req.t
Original file line number Diff line number Diff line change
Expand Up @@ -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'));

Expand All @@ -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])));
Expand Down

0 comments on commit ade7b08

Please sign in to comment.