From 64317a4b3d9ca668ad1acc57cc847eaad66d1cf1 Mon Sep 17 00:00:00 2001
From: Selva Nair <selva.nair@gmail.com>
Date: Thu, 2 May 2024 19:34:48 -0400
Subject: [PATCH] Validate the service pipe to avoid connecting to a bogus pipe

If an attacker with SeImeprsonatePrivilege manages to create a
namedpipe server with a name matching that used by the "Interactive
Service", the GUI connecting to it could allow the attacker to
impersonate the GUI user.

Fix by validating the service pipe by comparing the pid of the pipe
server with that of the "Interactive Service".

Note: GetNamedPipeServerProcessId() returns the pid of the process
that created the first instance of the pipe. So, this patch only
guards against a rogue pipe instance created before the service
has started. This has to work in combination with a patch for the
service that disallows creation of additional pipe instances when
the service is running.

CVE: CVE-2024-4877
Reported by: Zeze with TeamT5 <zeze7w@gmail.com>

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
---
 openvpn.c | 28 ++++++++++++++++++++++++++++
 service.c | 37 +++++++++++++++++++++++++++++++++++++
 service.h |  2 ++
 3 files changed, 67 insertions(+)

diff --git a/openvpn.c b/openvpn.c
index 9f832250..ac445c25 100644
--- a/openvpn.c
+++ b/openvpn.c
@@ -60,6 +60,7 @@
 #include "env_set.h"
 #include "echo.h"
 #include "pkcs11.h"
+#include "service.h"
 
 #define OPENVPN_SERVICE_PIPE_NAME_OVPN2 L"\\\\.\\pipe\\openvpn\\service"
 #define OPENVPN_SERVICE_PIPE_NAME_OVPN3 L"\\\\.\\pipe\\ovpnagent"
@@ -1874,6 +1875,26 @@ HandleServiceIO(DWORD err, DWORD bytes, LPOVERLAPPED lpo)
     /* Any error in the above call will get checked in next round */
 }
 
+static BOOL
+ValidatePipe(connection_t *c)
+{
+    ULONG ppid = 0, spid = 0;
+
+    if (!c->iserv.pipe)
+    {
+        return FALSE;
+    }
+    if (!GetNamedPipeServerProcessId(c->iserv.pipe, &ppid))
+    {
+        MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d Failed to get pipe server process id: (error = 0x%08x)",
+                              __func__, __LINE__, GetLastError());
+        return FALSE;
+    }
+    spid = GetServicePid();
+
+    return (ppid > 0) && (spid > 0) && (spid == ppid);
+}
+
 /*
  * Write size bytes in buf to the pipe with a timeout.
  * Retun value: TRUE on success FLASE on error
@@ -2737,6 +2758,13 @@ LaunchOpenVPN(connection_t *c)
     {
         BOOL res = FALSE;
 
+        if (!ValidatePipe(c))
+        {
+            CloseHandle(c->exit_event);
+            CloseServiceIO(&c->iserv);
+            goto out;
+        }
+
         if (o.ovpn_engine == OPENVPN_ENGINE_OVPN3)
         {
 #ifdef ENABLE_OVPN3
diff --git a/service.c b/service.c
index 9540ab48..070b1f87 100644
--- a/service.c
+++ b/service.c
@@ -188,3 +188,40 @@ StartAutomaticService(void)
     }
     return;
 }
+
+/*
+ * Returns the processId of the Interactive Service or zero on error
+ * which includes service not running.
+ */
+ULONG
+GetServicePid(void)
+{
+    SC_HANDLE schManager;
+    SC_HANDLE schService;
+    ULONG pid = 0;
+
+    schManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT);
+    if (schManager)
+    {
+        schService = OpenService(schManager, o.ovpn_engine == OPENVPN_ENGINE_OVPN3 ?
+                                 OPENVPN_SERVICE_NAME_OVPN3 : OPENVPN_SERVICE_NAME_OVPN2, SERVICE_QUERY_STATUS);
+        if (schService)
+        {
+            SERVICE_STATUS_PROCESS ssp = {0};
+            DWORD nbytes = 0;
+            if (QueryServiceStatusEx(schService, SC_STATUS_PROCESS_INFO, (BYTE*)&ssp, sizeof(ssp), &nbytes)
+                && ssp.dwCurrentState == SERVICE_RUNNING)
+            {
+                pid = ssp.dwProcessId;
+            }
+            CloseServiceHandle(schService);
+        }
+        CloseServiceHandle(schManager);
+    }
+    if (pid == 0)
+    {
+        MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d Failed to get service process id: (error = 0x%08x)",
+                              __func__, __LINE__, GetLastError());
+    }
+    return pid;
+}
diff --git a/service.h b/service.h
index d7f0de59..e17c2371 100644
--- a/service.h
+++ b/service.h
@@ -25,3 +25,5 @@ BOOL CheckIServiceStatus(BOOL warn);
 
 /* Attempt to start OpenVPN Automatc Service */
 void StartAutomaticService(void);
+/* Get the processId of the Interactive Service */
+ULONG GetServicePid(void);