On Sat, Nov 25, 2023 at 05:30:57PM +0000, Claus Assmann wrote:
> One of my regression tests crashes when using OpenSSL 3.2.0. I've
> tracked it down to:
>
> commit e4a94bcc77f3fda0f185e62a73a66d9b9b9388f5
> Fix a possible memory leak in dane_tlsa_add
> diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
> index 5314e1ec0d..70d3b17c19 100644
> --- a/ssl/ssl_lib.c
> +++ b/ssl/ssl_lib.c
> @@ -339,6 +341,7 @@ static int dane_tlsa_add(SSL_DANE *dane,
>
> if ((DANETLS_USAGE_BIT(usage) & DANETLS_TA_MASK) == 0) {
> X509_free(cert);
> + tlsa_free(t);
> break;
> }
>
> Now the question is: is this a bug in my application or in 3.2.0?
> Maybe someone who knows/understands the code can take a look?
> BTW: All the other added tlsa_free() calls are seemingly before
> a return statement.
>
Thanks for the report!
This is a freshly minted bug (just prior to the release) introducing a
double-free (rather than the intent of avoiding a memory leak):
commit 7f943d40bda4539d63da34ecfbbc8556f2603fb3
Author: Bernd Edlinger <bernd.edlinger@xxxxxxxxxx>
Date: Wed Nov 15 19:46:17 2023 +0100
Fix a possible memory leak in dane_tlsa_add
Several error cases leak either the X509 object
or the pkey or the danetls_record object.
Reviewed-by: Hugo Landau <hlandau@xxxxxxxxxxx>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@xxxxxxxxx>
Reviewed-by: Richard Levitte <levitte@xxxxxxxxxxx>
(Merged from https://github.com/openssl/openssl/pull/22743)
That tlsa_free(t) SHOULD NOT have been added, the record is still live
and used after the "break". The reviewers should have checked more
carefully... :-(
--
Viktor.
