| Message ID | 20230601095721.4065834-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Fix use-after-free with EVP_CIPHER_free | expand |
Hi, On 01/06/2023 11:57, Arne Schwabe wrote: > In many scenerios the context will still have a reference to the cipher, so scenerios -> scenarios > this use-after-free does not explode but it is still wrong. Good catch - glad we're so lucky :-) > > Change-Id: I59002d6613eaef36d5a47b20b56073e399cfa1df > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/crypto_openssl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index c2ac80b74..8fe56fc78 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -839,11 +839,12 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key, > crypto_msg(M_FATAL, "EVP cipher init #2"); > } > > - EVP_CIPHER_free(kt); > /* make sure we used a big enough key */ > ASSERT(EVP_CIPHER_CTX_key_length(ctx) <= EVP_CIPHER_key_length(kt)); > + EVP_CIPHER_free(kt); > } > > + This is not required - please remove it before merging. > int > cipher_ctx_iv_length(const EVP_CIPHER_CTX *ctx) > { Acked-by: Antonio Quartulli <a@unstable.cc>
I have not tested this beyond a simple "does it compile" (it does) - and
the fix is quite "obviously correct" - good find.
I have also removed the spurious extra blank line on merge.
Your patch has been applied to the master and release/2.6 branch
(relevant code is not part of 2.5).
commit 13f5e615310ea64ab69f521e622a10f2d0ad3f4e (master)
commit 205c66bd0ed2661c47b9fe7317089fbb09cc7aa4 (release/2.6)
Author: Arne Schwabe
Date: Thu Jun 1 11:57:21 2023 +0200
Fix use-after-free with EVP_CIPHER_free
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230601095721.4065834-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26735.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index c2ac80b74..8fe56fc78 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -839,11 +839,12 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key, crypto_msg(M_FATAL, "EVP cipher init #2"); } - EVP_CIPHER_free(kt); /* make sure we used a big enough key */ ASSERT(EVP_CIPHER_CTX_key_length(ctx) <= EVP_CIPHER_key_length(kt)); + EVP_CIPHER_free(kt); } + int cipher_ctx_iv_length(const EVP_CIPHER_CTX *ctx) {
In many scenerios the context will still have a reference to the cipher, so this use-after-free does not explode but it is still wrong. Change-Id: I59002d6613eaef36d5a47b20b56073e399cfa1df Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/crypto_openssl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)