Pass correct buffer size to GetModuleFileNameW()

From: Selva Nair <[email protected]...>

Fixes finding 5.6 of OSTIF/Quarkslab audit

---
 src/openvpn/win32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 0cbf5fd..9a03681 100644
@@ -1334,8 +1334,8 @@ win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel)
         goto out;
     }
 
-    status = GetModuleFileNameW(NULL, openvpnpath, sizeof(openvpnpath));
-    if (status == 0 || status == sizeof(openvpnpath))
+    status = GetModuleFileNameW(NULL, openvpnpath, _countof(openvpnpath));
+    if (status == 0 || status == _countof(openvpnpath))
     {
         msg(M_WARN|M_ERRNO, "block_dns: cannot get executable path");
         goto out;

2.1.4

ACK, thanks. (No tests run whatsoever, but we've had a discussion about that on the security@ lists, and there was agreement that _countof is the thing to use - just nobody did it before, so thanks again :-) ).

Your patch has been applied to the master and release/2.4 branch.

Release/2.3 has "MAX_PATH" instead of sizeof or _countof, which is actually correct (but in contrast, it has no error checking for GetModuleFileNameW()...)

commit 986b930862c7fecb2a42645f1dc23a53ab2cf6bb (master) commit 9092397ce02616b06c5c748de0b45775c7aa1e73 (release/2.4)

Author: Selva Nair
Date:   Thu May 11 21:32:34 2017 -0400

     Pass correct buffer size to GetModuleFileNameW()

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14647.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Oh, I missed that. Note to self: we should add error checking there. If the buffer is too small we will get a truncated but null terminated name, so that's not too bad. But if the call fails the buffer may contain random bytes.

Selva