diff mbox series

[Openvpn-devel,v1] iservice: make sure registry string is terminated

Message ID 20251123115851.19555-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] iservice: make sure registry string is terminated | expand

Commit Message

Gert Doering Nov. 23, 2025, 11:58 a.m. UTC 
From: Heiko Hund <heiko@ist.eigentlich.net>

When reading the interface domains from the registry, check that the
string is zero terminated, since the code in GetItfDnsDomains depends
on the fact when doing size calculations during the conversion.

Reported-by: Marc Heuse <marc@srlabs.de>
Reported-by: stephan@srlabs.de
Change-Id: Icaeca22bdbd8ead0cb12345b1bcc2b5c0f46236f
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/+/1392
---

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

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Nov. 23, 2025, 1:28 p.m. UTC | #1 
Trivial enough.

Not sure we really *need* this - the documentation for RegGetValueW()
states

    If the data is a string, the function checks for a terminating
    null character. If one is not found, the string is stored with
    a null terminator if the buffer is large enough to accommodate
    the extra character. Otherwise, the function fails and returns
    ERROR_MORE_DATA.

which reads to me like "either we get 'err' or it is always null 
terminated", so the "if (!err)" part would catch this anyway.

I have no way to actually test this beyond "does it compile" - which
it does.

Your patch has been applied to the master branch.

commit a1c41b0acb6905506ad35e339396206653fee5cc
Author: Heiko Hund
Date:   Sun Nov 23 12:58:46 2025 +0100

     iservice: make sure registry string is terminated

     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/+/1392
     Message-Id: <20251123115851.19555-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34610.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 f31a8a9..ec80a30 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2163,7 +2163,7 @@ 
     {
         *size = buf_size;
         err = RegGetValueW(itf, NULL, values[i], RRF_RT_REG_SZ, NULL, (PBYTE)domains, size);
-        if (!err && *size > one_glyph && wcschr(domains, '.'))
+        if (!err && *size > one_glyph && domains[(*size / one_glyph) - 1] == '\0' && wcschr(domains, '.'))
         {
             /*
              * Found domain(s), now convert them: