[Openvpn-devel,v4] ssl_openssl.c: Prevent potential double-free

Message ID 20250417134636.21279-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v4] ssl_openssl.c: Prevent potential double-free | expand

Commit Message

Gert Doering April 17, 2025, 1:46 p.m. UTC
From: Lev Stipakov <lev@openvpn.net>

Fixes a potential double-free issue in tls_ctx_load_cert_uri()
by explicitly nullifying the pointer immediately after calling OSSL_STORE_INFO_free(info).

This ensures that subsequent cleanup won't attempt to free the same pointer again.

GitHub: #726

Change-Id: I4507be07cd5573b2117e837ef03187535a38a4b1
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <antonio@mandelbit.com>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/942
This mail reflects revision 4 of this Change.

Acked-by according to Gerrit (reflected above):
Antonio Quartulli <antonio@mandelbit.com>

Comments

Gert Doering April 17, 2025, 2:04 p.m. UTC | #1
Not tested, as the offending code flow is quite obvious "in hindsight", and
making the pointer NULL after free() is a sufficient safeguard against
double free().  A slightly more readable construction could have used
to different "info" variables for the two different scopes, each with
its own individual free(), exactly one per variable... but that's a
larger change.

Your patch has been applied to the master branch.

Application to 2.6 and earlier is not needed, the offending code is new
(e9ad1b3 or 3512e8d).

commit f7aedca70e24e9a35f0cbd33d1aa708b4daf0055
Author: Lev Stipakov
Date:   Thu Apr 17 15:46:30 2025 +0200

     ssl_openssl.c: Prevent potential double-free

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Antonio Quartulli <antonio@mandelbit.com>
     Message-Id: <20250417134636.21279-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31478.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d1d5d3e..f7be50c 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1152,6 +1152,7 @@ 
         goto end;
     }
     OSSL_STORE_INFO_free(info);
+    info = NULL;
 
     /* iterate through the store and add extra certificates if any to the chain */
     while (!OSSL_STORE_eof(store_ctx))
@@ -1170,6 +1171,7 @@ 
             break;
         }
         OSSL_STORE_INFO_free(info);
+        info = NULL;
     }
 
 end: