[Openvpn-devel] [XS] Change in openvpn[master]: Fix check_session_buf_not_used using wrong index

2023-12-02 Thread cron2 (Code Review) 
cron2 has uploaded a new patch set (#2) to the change originally created by 
plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/459?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld


Change subject: Fix check_session_buf_not_used using wrong index
..

Fix check_session_buf_not_used using wrong index

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 was not doing anything
really useful with i instead of j, at least it was not crashing or
anything similar.

Noticed-By: Jon Williams (braindead-bf) on Github issue #449
Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Signed-off-by: Arne Schwabe 
Acked-by: Frank Lichtenheld 
Message-Id: <20231128104359.62967-1-fr...@lichtenheld.com>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27576.html
Signed-off-by: Gert Doering 
---
M src/openvpn/ssl.c
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/59/459/2

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])",

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/459?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Gerrit-Change-Number: 459
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-MessageType: newpatchset
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Fix check_session_buf_not_used using wrong index

2023-12-02 Thread cron2 (Code Review) 
cron2 has submitted this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/459?usp=email )

Change subject: Fix check_session_buf_not_used using wrong index
..

Fix check_session_buf_not_used using wrong index

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 was not doing anything
really useful with i instead of j, at least it was not crashing or
anything similar.

Noticed-By: Jon Williams (braindead-bf) on Github issue #449
Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Signed-off-by: Arne Schwabe 
Acked-by: Frank Lichtenheld 
Message-Id: <20231128104359.62967-1-fr...@lichtenheld.com>
URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27576.html
Signed-off-by: Gert Doering 
---
M src/openvpn/ssl.c
1 file changed, 1 insertion(+), 1 deletion(-)




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])",

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/459?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Gerrit-Change-Number: 459
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-MessageType: merged
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Fix check_session_buf_not_used using wrong index

2023-11-28 Thread flichtenheld (Code Review) 
Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/459?usp=email )

Change subject: Fix check_session_buf_not_used using wrong index
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/459?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Gerrit-Change-Number: 459
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Comment-Date: Tue, 28 Nov 2023 10:43:23 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Fix check_session_buf_not_used using wrong index

2023-11-22 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

http://gerrit.openvpn.net/c/openvpn/+/459?usp=email

to review the following change.


Change subject: Fix check_session_buf_not_used using wrong index
..

Fix check_session_buf_not_used using wrong index

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 
---
M src/openvpn/ssl.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/59/459/1

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])",

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/459?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia3d5b4946138df322ebcd9e9e77d04328dacbc5d
Gerrit-Change-Number: 459
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos 
Gerrit-Reviewer: flichtenheld 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-MessageType: newchange
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel