[Openvpn-devel] Fix possible access of uninitialized pipe handles

Message ID 1582163803-3342-1-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Fix possible access of uninitialized pipe handles | expand

Commit Message

Selva Nair Feb. 19, 2020, 2:56 p.m. UTC
From: Selva Nair <selva.nair@gmail.com>

Compile time warning for openvpnserv.exe
interactive.c: In function ‘RunOpenvpn’:
interactive.c:160:27: warning: ‘svc_pipe’ may be used uninitialized in
this function [-Wmaybe-uninitialized]

When RunOpenvpn exits early due to errors, uninitialized svc_pipe and
ovpn_pipe vars could get passed to CloseHandleEx(). Fix by initializing
to NULL.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpnserv/interactive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lev Stipakov Feb. 19, 2020, 10:24 p.m. UTC | #1
Strangely, I do not see this warning (unlike another one about error
in common.c)
with GCC 7.3 despite adding -O1 and -Wmaybe-uninitialized.

Change looks reasonable, compiled with MSVC and MinGW.

Acked-by: Lev Stipakov <lstipakov@gmail.com>
Selva Nair Feb. 20, 2020, 3:27 a.m. UTC | #2
Hi

On Thu, Feb 20, 2020 at 4:24 AM Lev Stipakov <lstipakov@gmail.com> wrote:
>
> Strangely, I do not see this warning (unlike another one about error
> in common.c)
> with GCC 7.3 despite adding -O1 and -Wmaybe-uninitialized.

I saw it on the travis build. With gcc 7.3, for some reason, -O1
doesn't show it but -O2 or higher does. Some older versions of gcc
seem to show it only with require -O3 or higher!

But the potential for attempting to close wrong handles looks real.

Thanks,

Selva

Patch

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 710f9c7..8da49be 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1468,7 +1468,7 @@  static DWORD WINAPI
 RunOpenvpn(LPVOID p)
 {
     HANDLE pipe = p;
-    HANDLE ovpn_pipe, svc_pipe;
+    HANDLE ovpn_pipe = NULL, svc_pipe = NULL;
     PTOKEN_USER svc_user = NULL, ovpn_user = NULL;
     HANDLE svc_token = NULL, imp_token = NULL, pri_token = NULL;
     HANDLE stdin_read = NULL, stdin_write = NULL;