diff mbox series

[Openvpn-devel,v6] win: calculate address string buffer size

Message ID 20250324083350.4019-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v6] win: calculate address string buffer size | expand

Commit Message

Gert Doering March 24, 2025, 8:33 a.m. UTC 
From: Heiko Hund <heiko@ist.eigentlich.net>

Instead of making the string buffer statically sized for a max. of
four addresses, calculate it to hold up to the max number of addresses
a dns_cfg_message_t can hold (currently four as well). Improves the code
so that it doesn't rely on the addresses never being more than four in
the future.

Change-Id: I23710b1f5b2122ec1f14465911836c0f0afa9c64
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
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/+/908
This mail reflects revision 6 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering March 24, 2025, 9:20 a.m. UTC | #1 
Thanks for future-proofing this part of the code - it's fine today, but
we might bump the number of allowed DNS addresses to "8" one day, and
then the old code was at-risk for overflows...

I haven't actually tested it, just stared at it for a bit, asked mingw
("no warnings") and we have Frank's +2 in gerrit.

Your patch has been applied to the master branch.

commit 8ea5debaea01da5fee56fbad56b50820c1beee92
Author: Heiko Hund
Date:   Mon Mar 24 09:33:44 2025 +0100

     win: calculate address string buffer size

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20250324083350.4019-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31196.html
     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 abbc916..100c69a 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1857,9 +1857,10 @@ 
     int addr_len = msg->addr_len;
 
     /* sanity check */
-    if (addr_len > _countof(msg->addr))
+    const size_t max_addrs = _countof(msg->addr);
+    if (addr_len > max_addrs)
     {
-        addr_len = _countof(msg->addr);
+        addr_len = max_addrs;
     }
 
     if (!msg->iface.name[0]) /* interface name is required */
@@ -1909,7 +1910,7 @@ 
     if (msg->addr_len > 0)
     {
         /* prepare the comma separated address list */
-        CHAR addrs[256]; /* large enough to hold four IPv4 / IPv6 address strings */
+        CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */
         size_t offset = 0;
         for (int i = 0; i < addr_len; ++i)
         {