diff mbox series

[Openvpn-devel,v2] Do not check key_state buffers that are in S_UNDEF state

Message ID 20231115103331.18050-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v2] Do not check key_state buffers that are in S_UNDEF state | expand

Commit Message

Gert Doering Nov. 15, 2023, 10:33 a.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

When a key_state is in S_UNDEF the send_reliable is not initialised. So
checking it might access invalid memory or null pointers.

Github: fixes OpenVPN/openvpn#449

Change-Id: I226a73d47a2b1b29f7ec175ce23a806593abc2ac
[a@unstable.cc: add check for !send_reliable and message]
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/426
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Nov. 15, 2023, 2:31 p.m. UTC | #1 
I have tested the "lite" version of that patch (only the S_UNDEF patch)
for 30+ hours in a scenario that managed to crash after a few hours without
it, so am reasonably confident it fixes the 2.6.7 crashes observed - in
case there *are* other cases, we added a M_FATAL so we stop with a debug
note, and do not SIGSEGV.  This really is a "can't happen", but *if* it
does, we want to know.

Your patch has been applied to the master and release/2.6 branch.

commit a903ebe9361d451daee71c225e141f4e1b67107d (master)
commit b90ec6dabfb151dd93ef00081bbc3f55e7d3450f (release/2.6)
Author: Arne Schwabe
Date:   Wed Nov 15 11:33:31 2023 +0100

     Do not check key_state buffers that are in S_UNDEF state

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20231115103331.18050-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27401.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index cee4afe..b4cd8f5 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3189,6 +3189,22 @@ 
     for (int i = 0; i < KS_SIZE; i++)
     {
         struct key_state *ks = &session->key[i];
+        if (ks->state == S_UNDEF)
+        {
+            continue;
+        }
+
+        /* we don't expect send_reliable to be NULL when state is
+         * not S_UNDEF, but people have reported crashes nonetheless,
+         * therefore we better catch this event, report and exit.
+         */
+        if (!ks->send_reliable)
+        {
+            msg(M_FATAL, "ERROR: session->key[%d]->send_reliable is NULL "
+                "while key state is %s. Exiting.",
+                i, state_name(ks->state));
+        }
+
         for (int j = 0; j < ks->send_reliable->size; j++)
         {
             if (ks->send_reliable->array[i].buf.data == dataptr)