Skip to contentSkip to conversation

Fix use-after-free in tls_ctx_use_management_external_key

PATCH In review Steffan Karger 3 participants 3 messages Oct 7, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix use-after-free in tls_ctx_use_management_external_key

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.

---
 src/openvpn/ssl_openssl.c | 1 -
 1 file changed, 1 deletion(-)
src/openvpn/ssl_openssl.c +0 -1
index fe4db604..7532773e 100644
@@ -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)
     {
Signature · 1 line
2.17.1
Acked-by on v1 1/1
#1 lore raw

Am 07.10.18 um 13:00 schrieb Steffan Karger:

Quoted 10 lines

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

#2 lore raw
In reply to Steffan Karger

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 <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering