diff mbox series

[Openvpn-devel,v2] openvpnserv: Address some uninitVariable warnings from cppcheck

Message ID 20260513150902.27447-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] openvpnserv: Address some uninitVariable warnings from cppcheck |

Commit Message

Gert Doering May 13, 2026, 3:08 p.m. UTC 
  From: Frank Lichtenheld <frank@lichtenheld.com>

In the first case this is about helping cppcheck
remember that msg->addr_len and addr_len are the same
thing, but we use them in confusing ways.

In the second case there is indeed a theoretical
code path where we use an uninitialized buffer. So
make the code safer.

Change-Id: Ida6d4fa8c5c5ffbd7909d6afd51b1b6f32ca2d9f
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Heiko Hund <heiko@openvpn.net>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1674
---

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/+/1674
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Heiko Hund <heiko@openvpn.net>

Comments

Gert Doering May 14, 2026, 10:37 a.m. UTC | #1 
Thanks for looking into this, thanks Heiko for review and +2.

I did some stare-at-code (looks good).  BB did lots of test compiles 
and test runs - some red, due to spurious "test 1* failures", which
are unrelated to this (unix platforms fail, windows-only code changes).

Understanding the second one took me a bit of time - basically, if
there are no addresses whatsoever, addr_list[] will be uninitialized
(because no strcpy()) and then pased on to SetNrptRule().  Not sure if
this can happen in practice, not now it's clear "it can't".

Your patch has been applied to the master and release/2.7 branch (bugfix).

commit 50bcb9206b1dc3100a6b20cf569c4caf9c9105e3 (master)
commit d3eb09b559ef7832a236934363e3c0aa5d855307 (release/2.7)
Author: Frank Lichtenheld
Date:   Wed May 13 17:08:57 2026 +0200

     openvpnserv: Address some uninitVariable warnings from cppcheck

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Heiko Hund <heiko@openvpn.net>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1674
     Message-Id: <20260513150902.27447-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36908.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 bd310e4..781391c 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1876,7 +1876,7 @@ 
         return err; /* job done */
     }
 
-    if (msg->addr_len > 0)
+    if (addr_len > 0)
     {
         /* prepare the comma separated address list */
         /* cannot use max_addrs here as that is not considered compile
@@ -2647,25 +2647,28 @@ 
         free(wide_search_domains);
     }
 
-    /* Create address string list */
-    CHAR addr_list[NRPT_ADDR_NUM * NRPT_ADDR_SIZE];
-    PSTR pos = addr_list;
-    for (int i = 0; i < NRPT_ADDR_NUM && addresses[i][0]; ++i)
+    if (addresses[0][0])
     {
-        if (i != 0)
+        /* Create address string list */
+        CHAR addr_list[NRPT_ADDR_NUM * NRPT_ADDR_SIZE];
+        PSTR pos = addr_list;
+        for (int i = 0; i < NRPT_ADDR_NUM && addresses[i][0]; ++i)
         {
-            *pos++ = ';';
+            if (i != 0)
+            {
+                *pos++ = ';';
+            }
+            strcpy(pos, addresses[i]);
+            pos += strlen(pos);
         }
-        strcpy(pos, addresses[i]);
-        pos += strlen(pos);
-    }
 
-    WCHAR subkey[MAX_PATH];
-    swprintf(subkey, _countof(subkey), L"OpenVPNDNSRouting-%lu", ovpn_pid);
-    err = SetNrptRule(nrpt_key, subkey, addr_list, wide_domains, dom_size, dnssec);
-    if (err)
-    {
-        MsgToEventLog(M_ERR, L"%S: failed to set rule %s (%lu)", __func__, subkey, err);
+        WCHAR subkey[MAX_PATH];
+        swprintf(subkey, _countof(subkey), L"OpenVPNDNSRouting-%lu", ovpn_pid);
+        err = SetNrptRule(nrpt_key, subkey, addr_list, wide_domains, dom_size, dnssec);
+        if (err)
+        {
+            MsgToEventLog(M_ERR, L"%S: failed to set rule %s (%lu)", __func__, subkey, err);
+        }
     }
 
     if (domains[0])