From 4547dd95fc343f90f49bd2770d9f7946f3a1bbbe 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>

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

diff --git a/openvpn.c b/openvpn.c
index 9f83225..8debf90 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 9540ab4..13b1f4e 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 d7f0de5..471902f 100644
--- a/service.h
+++ b/service.h
@@ -25,3 +25,6 @@ BOOL CheckIServiceStatus(BOOL warn);
 
 /* Attempt to start OpenVPN Automatc Service */
 void StartAutomaticService(void);
+
+/* Get the processId of the Interactive Service */
+ULONG GetServicePid(void);