Skip to content

Commit

Permalink
Fix using to_link buffer after freed
Browse files Browse the repository at this point in the history
When I refactored the tls_state_change method in
9a7b95f 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 <[email protected]>
Acked-by: David Sommerseth <[email protected]>
Acked-by: Heiko Hund <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/search?l=mid&[email protected]
Signed-off-by: Gert Doering <[email protected]>
(cherry picked from commit 57a5cd1)
  • Loading branch information
schwabe authored and cron2 committed Nov 8, 2023
1 parent f09d750 commit a0afe03
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/openvpn/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2903,7 +2903,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 */
Expand Down

0 comments on commit a0afe03

Please sign in to comment.