diff mbox series

[Openvpn-devel,v1] Fix check_session_buf_not_used using wrong index

Message ID 20231128104359.62967-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel,v1] Fix check_session_buf_not_used using wrong index | expand

Commit Message

Frank Lichtenheld Nov. 28, 2023, 10:43 a.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

The inner loop used i instead of j when iterating through the buffers.
Since i is always between 0 and 2 and ks->send_reliable->size is
(when it is defined) always 6 (TLS_RELIABLE_N_SEND_BUFFERS) this does not
cause an index of out bounds. So while the check is not doing anything
really useful with i instead of  j, it at least is not crashing or
anything similar.

Noticed-By: Jon Williams (braindead-bf) on Github issue #449
Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
---

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/+/459
This mail reflects revision 1 of this Change.
Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering Dec. 2, 2023, 3:56 p.m. UTC | #1 
Indeed, that function seems to be slightly cursed... and this is the
right fix.  For this bug.

Tested more thoroughly :-) (full server test runs + GHA).

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

commit 59551b93cdb55397d63b2fe58ad99612821c0faf (master)
commit 5def8d935335619b16452b56b332d06f4d621d75 (release/2.6)
Author: Arne Schwabe
Date:   Tue Nov 28 11:43:59 2023 +0100

     Fix check_session_buf_not_used using wrong index

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20231128104359.62967-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27576.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 400230c..b5d24b5 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3207,7 +3207,7 @@ 
 
         for (int j = 0; j < ks->send_reliable->size; j++)
         {
-            if (ks->send_reliable->array[i].buf.data == dataptr)
+            if (ks->send_reliable->array[j].buf.data == dataptr)
             {
                 msg(M_INFO, "Warning buffer of freed TLS session is still in"
                     " use (session->key[%d].send_reliable->array[%d])",