From: Arne Schwabe <arne@rfc2549.org>
Date: Wed, 25 Oct 2023 15:46:24 +0000 (+0200)
Subject: Double check that we do not use a freed buffer when freeing a session
X-Git-Tag: v2.7_alpha1~382
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f09d75083e8d7f6db3d90a25480e44de91bd8154;p=thirdparty%2Fopenvpn.git

Double check that we do not use a freed buffer when freeing a session

This is a find cases where the session already has planned to send out
a packet but encounters some other errors that invalidate the session,
setting it to S_ERROR and leaving the buffer behind.

This will detect and clear that to_link buffer in that case.

Change-Id: I5ffb41bed1c9237946b13d787eb4c4013e0bec68
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-2-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20231108124947.76816-2-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit cd4d819c99266fa727c294225cafdb4ae331d02e)
---

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 5e6205cc2..e15f951d6 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3155,6 +3155,53 @@ tls_process(struct tls_multi *multi,
     return false;
 }
 
+
+/**
+ * This is a safe guard function to double check that a buffer from a session is
+ * not used in a session to avoid a use after free.
+ *
+ * @param to_link
+ * @param session
+ */
+static void
+check_session_buf_not_used(struct buffer *to_link, struct tls_session *session)
+{
+    uint8_t *dataptr = to_link->data;
+    if (!dataptr)
+    {
+        return;
+    }
+
+    /* Checks buffers in tls_wrap */
+    if (session->tls_wrap.work.data == dataptr)
+    {
+        msg(M_INFO, "Warning buffer of freed TLS session is "
+            "still in use (tls_wrap.work.data)");
+        goto used;
+    }
+
+    for (int i = 0; i < KS_SIZE; i++)
+    {
+        struct key_state *ks = &session->key[i];
+        for (int j = 0; j < ks->send_reliable->size; j++)
+        {
+            if (ks->send_reliable->array[i].buf.data == dataptr)
+            {
+                msg(M_INFO, "Warning buffer of freed TLS session is still in"
+                    " use (session->key[%d].send_reliable->array[%d])",
+                    i, j);
+
+                goto used;
+            }
+        }
+    }
+    return;
+
+used:
+    to_link->len = 0;
+    to_link->data = 0;
+    /* for debugging, you can add an ASSERT(0); here to trigger an abort */
+}
 /*
  * Called by the top-level event loop.
  *
@@ -3253,6 +3300,7 @@ tls_multi_process(struct tls_multi *multi,
                 }
                 else
                 {
+                    check_session_buf_not_used(to_link, session);
                     reset_session(multi, session);
                 }
             }