Vulnerability: openvpnserv.exe (aka the interactive service helper) in OpenVPN 2.4.x before 2.4.6 allows a local attacker to cause a double-free of memory by sending a malformed request to the interactive service. This could cause a denial-of-service through memory corruption or possibly have unspecified other impact including privilege escalation.
Vulnerability ID: CVE-2018-9336
CWE: CWE-415
Severity score: 6.4
Project: https://github.com/OpenVPN/openvpn
Published: 2018-05-01
File: src/openvpnserv/interactive.c
Commit message: Fix potential double-free() in Interactive Service (CVE-2018-9336)

Malformed input data on the service pipe towards the OpenVPN interactive
service (normally used by the OpenVPN GUI to request openvpn instances
from the service) can result in a double free() in the error handling code.

This usually only leads to a process crash (DoS by an unprivileged local
account) but since it could possibly lead to memory corruption if
happening while multiple other threads are active at the same time,
CVE-2018-9336 has been assigned to acknowledge this risk.

Fix by ensuring that sud->directory is set to NULL in GetStartUpData()
for all error cases (thus not being free()ed in FreeStartupData()).

Rewrite control flow to use explicit error label for error exit.

Discovered and reported by Jacob Baines <jbaines@tenable.com>.

CVE: 2018-9336

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20180414072617.25075-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20180414072617.25075-1-gert@greenie.muc.de

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit author: Gert Doering

Vulnerable version (before fix):
 static BOOL GetStartupData(HANDLE pipe, STARTUP_DATA *sud) {     size_t size, len;     BOOL ret = FALSE;     WCHAR *data = NULL;     DWORD bytes, read;  GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_SYSERR, TEXT("PeekNamedPipeAsync failed"));         ReturnLastError(pipe, L"PeekNamedPipeAsync");         goto out;     }      size = bytes / sizeof(*data);     if (size == 0)     {         MsgToEventLog(M_SYSERR, TEXT("malformed startup data: 1 byte received"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto out;     }      data = malloc(bytes);     if (data == NULL)     {         MsgToEventLog(M_SYSERR, TEXT("malloc failed"));         ReturnLastError(pipe, L"malloc");         goto out;     }      read = ReadPipeAsync(pipe, data, bytes, 1, &exit_event);     if (bytes != read)     {         MsgToEventLog(M_SYSERR, TEXT("ReadPipeAsync failed"));         ReturnLastError(pipe, L"ReadPipeAsync");         goto out;     }      if (data[size - 1] != 0)     {         MsgToEventLog(M_ERR, TEXT("Startup data is not NULL terminated"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto out;     }      sud->directory = data; GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_ERR, TEXT("Startup data ends at working directory"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto out;     }      sud->options = sud->directory + len; GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_ERR, TEXT("Startup data ends at command line options"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto out;     }      sud->std_input = sud->options + len;     data = NULL; /* don't free data */     ret = TRUE;  out:     free(data);     return ret; }  

Fixed version (after fix):
 static BOOL GetStartupData(HANDLE pipe, STARTUP_DATA *sud) {     size_t size, len;     WCHAR *data = NULL;     DWORD bytes, read;  GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_SYSERR, TEXT("PeekNamedPipeAsync failed"));         ReturnLastError(pipe, L"PeekNamedPipeAsync");         goto err;     }      size = bytes / sizeof(*data);     if (size == 0)     {         MsgToEventLog(M_SYSERR, TEXT("malformed startup data: 1 byte received"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto err;     }      data = malloc(bytes);     if (data == NULL)     {         MsgToEventLog(M_SYSERR, TEXT("malloc failed"));         ReturnLastError(pipe, L"malloc");         goto err;     }      read = ReadPipeAsync(pipe, data, bytes, 1, &exit_event);     if (bytes != read)     {         MsgToEventLog(M_SYSERR, TEXT("ReadPipeAsync failed"));         ReturnLastError(pipe, L"ReadPipeAsync");         goto err;     }      if (data[size - 1] != 0)     {         MsgToEventLog(M_ERR, TEXT("Startup data is not NULL terminated"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto err;     }      sud->directory = data; GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_ERR, TEXT("Startup data ends at working directory"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto err;     }      sud->options = sud->directory + len; GetStartupData(HANDLE pipe, STARTUP_DATA *sud)     {         MsgToEventLog(M_ERR, TEXT("Startup data ends at command line options"));         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);         goto err;     }      sud->std_input = sud->options + len;     return TRUE;  err:     sud->directory = NULL;		/* caller must not free() */     free(data);     return FALSE; }  

Patch:
patch: @@ -453,7 +453,6 @@ static BOOL
 GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
 {
     size_t size, len;
-    BOOL ret = FALSE;
     WCHAR *data = NULL;
     DWORD bytes, read;
 
@@ -462,38 +461,38 @@ GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
     {
         MsgToEventLog(M_SYSERR, TEXT("PeekNamedPipeAsync failed"));
         ReturnLastError(pipe, L"PeekNamedPipeAsync");
-        goto out;
+        goto err;
     }
 
     size = bytes / sizeof(*data);
     if (size == 0)
     {
         MsgToEventLog(M_SYSERR, TEXT("malformed startup data: 1 byte received"));
         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);
-        goto out;
+        goto err;
     }
 
     data = malloc(bytes);
     if (data == NULL)
     {
         MsgToEventLog(M_SYSERR, TEXT("malloc failed"));
         ReturnLastError(pipe, L"malloc");
-        goto out;
+        goto err;
     }
 
     read = ReadPipeAsync(pipe, data, bytes, 1, &exit_event);
     if (bytes != read)
     {
         MsgToEventLog(M_SYSERR, TEXT("ReadPipeAsync failed"));
         ReturnLastError(pipe, L"ReadPipeAsync");
-        goto out;
+        goto err;
     }
 
     if (data[size - 1] != 0)
     {
         MsgToEventLog(M_ERR, TEXT("Startup data is not NULL terminated"));
         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);
-        goto out;
+        goto err;
     }
 
     sud->directory = data;
@@ -503,7 +502,7 @@ GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
     {
         MsgToEventLog(M_ERR, TEXT("Startup data ends at working directory"));
         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);
-        goto out;
+        goto err;
     }
 
     sud->options = sud->directory + len;
@@ -513,16 +512,16 @@ GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
     {
         MsgToEventLog(M_ERR, TEXT("Startup data ends at command line options"));
         ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData", 1, &exit_event);
-        goto out;
+        goto err;
     }
 
     sud->std_input = sud->options + len;
-    data = NULL; /* don't free data */
-    ret = TRUE;
+    return TRUE;
 
-out:
+err:
+    sud->directory = NULL;		/* caller must not free() */
     free(data);
-    return ret;
+    return FALSE;
 }
path: src/openvpnserv/interactive.c