diff mbox series

[Openvpn-devel,v2] iservice: fix calculation of converted domains size

Message ID 20251126103427.4085-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v2] iservice: fix calculation of converted domains size | expand

Commit Message

Gert Doering Nov. 26, 2025, 10:34 a.m. UTC 
From: Heiko Hund <heiko@ist.eigentlich.net>

To keep track of how much of the buffer is already used, the difference
of the current position and the start of the buffer needs to be
multiplied with the size of a character / glyph to bet the byte count,
with which calculations are done further down below.

Reported-by: Marc Heuse <marc@srlabs.de>
Reported-by: stephan@srlabs.de
Change-Id: I16f9426e57f4802ba038ab51f5b70161464b9428
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1390
---

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/+/1390
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. 26, 2025, 11:01 a.m. UTC | #1 
Stared long and hard at the code.  Found lots of "this cannot work, really?"
staring at it, that are all getting fixed in additional single-line patches
later on - this is not a useful patch granularity, sending 8 patches to
fix like 6 single-line bugs.  Reviewing and testing something where the
starting code is broken to no end and a single patch will not bring it 
into a correct state is not a good use of my time.

This said, this single-line change (plus a bit of movearound in preparation
for the next patch) does what it says, calculate "converted_size"
correctly, in bytes ("_len" in glyphs, "_size" = in bytes).

Your patch has been applied to the master branch.

commit ac6c9d65701ed6d41691497846a47cb4e72b2878
Author: Heiko Hund
Date:   Wed Nov 26 11:34:21 2025 +0100

     iservice: fix calculation of converted domains size

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1390
     Message-Id: <20251126103427.4085-1-gert@greenie.muc.de>
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 07ca7c9..9c533c1 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2181,8 +2181,11 @@ 
                     *comma = '\0';
                 }
 
-                /* Ignore itf domains which match a pushed search domain */
                 size_t domain_len = wcslen(pos);
+                size_t domain_size = domain_len * one_glyph;
+                size_t converted_size = (pos - domains) * one_glyph;
+
+                /* Ignore itf domains which match a pushed search domain */
                 if (ListContainsDomain(search_domains, pos, domain_len))
                 {
                     if (comma)
@@ -2199,11 +2202,14 @@ 
                     }
                 }
 
-                /* Check for enough space to convert this domain */
-                domain_len += 1; /* leading dot */
-                size_t converted_size = pos - domains;
-                size_t domain_size = domain_len * one_glyph;
+                /* Add space for the leading dot */
+                domain_len += 1;
+                domain_size += one_glyph;
+
+                /* Space for the terminating zeros */
                 size_t extra_size = 2 * one_glyph;
+
+                /* Check for enough space to convert this domain */
                 if (converted_size + domain_size + extra_size > buf_size)
                 {
                     /* Domain doesn't fit, bad luck if it's the first one */