diff options
| author | Arne Schwabe | 2023-10-27 14:19:37 +0200 |
|---|---|---|
| committer | Gert Doering | 2023-11-08 13:58:04 +0100 |
| commit | 57a5cd1e12f193927c9b7429f8778fec7e04c50a (patch) | |
| tree | 4db8dc99df21f7a41c2c5c37cdcc1c1bdb6d0118 | |
| parent | cd4d819c99266fa727c294225cafdb4ae331d02e (diff) | |
| download | openvpn-57a5cd1e12f193927c9b7429f8778fec7e04c50a.zip openvpn-57a5cd1e12f193927c9b7429f8778fec7e04c50a.tar.gz | |
Fix using to_link buffer after freed
When I refactored the tls_state_change method in
9a7b95fda5 I accidentally changed a break into
a return true while it should return a false.
The code here is extremely fragile in the sense
that it assumes that settings a keystate to S_ERROR
cannot have any outgoing buffer or we will have a
use after free. The previous break and now restored
return false ensure this by skipping any further
tls_process_state loops that might set to ks->S_ERROR
and ensure that the to_link is sent out and cleared
before having more loops in tls_state_change.
CVE: 2023-46850
This affects everyone, even with tls-auth/tls-crypt enabled.
Change-Id: I2a0f1c665d992da8e24a421ff0ddcb40f7945ea8
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20231108124947.76816-3-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20231108124947.76816-3-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
| -rw-r--r-- | src/openvpn/ssl.c | 8 |
1 files changed, 7 insertions, 1 deletions
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 022dc79..9e0ad02 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2906,7 +2906,13 @@ tls_process_state(struct tls_multi *multi, CONTROL_SEND_ACK_MAX, true); *to_link = b; dmsg(D_TLS_DEBUG, "Reliable -> TCP/UDP"); - return true; + + /* This changed the state of the outgoing buffer. In order to avoid + * running this function again/further and invalidating the key_state + * buffer and accessing the buffer that is now in to_link after it being + * freed for a potential error, we shortcircuit exiting of the outer + * process here. */ + return false; } /* Write incoming ciphertext to TLS object */ |