[Openvpn-devel] Fix use-after-free in tls_ctx_use_management_external_key

Message ID 20181007100032.17060-1-steffan@karger.me
State Accepted
Headers show
Series [Openvpn-devel] Fix use-after-free in tls_ctx_use_management_external_key | expand

Commit Message

Steffan Karger Oct. 6, 2018, 11 p.m. UTC
Commit 98bfeeb4 changed our openssl backend implementation of
tls_ctx_use_management_external_key() to no longer use
tls_ctx_load_cert_file_and_copy(), but still free'd 'cert'. Which it no
longer should do. Credits go to Arne for spotting the issue (even though
it was missed during the review).

The offending commit is only recently applied to the master branch, so was
never part of a OpenVPN release. For that reason I did not do full impact
analysis.

Signed-off-by: Steffan Karger <steffan@karger.me>
---
 src/openvpn/ssl_openssl.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Arne Schwabe Oct. 7, 2018, 12:46 a.m. UTC | #1
Am 07.10.18 um 13:00 schrieb Steffan Karger:
> Commit 98bfeeb4 changed our openssl backend implementation of
> tls_ctx_use_management_external_key() to no longer use
> tls_ctx_load_cert_file_and_copy(), but still free'd 'cert'. Which it no
> longer should do. Credits go to Arne for spotting the issue (even though
> it was missed during the review).
> 
> The offending commit is only recently applied to the master branch, so was
> never part of a OpenVPN release. For that reason I did not do full impact
> analysis.
> 


ACK. I already tested this on my on side.

Acked-By: Arne Schwabe


Arne
Gert Doering Oct. 7, 2018, 1:11 a.m. UTC | #2
Your patch has been applied to the master branch.

commit 68e0b9db253ff0437047d6a5377eeec6002873f8
Author: Steffan Karger
Date:   Sun Oct 7 12:00:32 2018 +0200

     Fix use-after-free in tls_ctx_use_management_external_key

     Signed-off-by: Steffan Karger <steffan@karger.me>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20181007100032.17060-1-steffan@karger.me>
     URL: https://www.mail-archive.com/search?l=mid&q=20181007100032.17060-1-steffan@karger.me
     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 fe4db604..7532773e 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1291,7 +1291,6 @@  tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
     /* get the public key */
     EVP_PKEY *pkey = X509_get0_pubkey(cert);
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
-    X509_free(cert);
 
     if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
     {