[Openvpn-devel] CVE fix for Windows in 2.5, 2.6, 2.7_rc3

Message ID aSr4hZgQWtFl-qLL@greenie.muc.de
State New
Headers show
Series [Openvpn-devel] CVE fix for Windows in 2.5, 2.6, 2.7_rc3 | expand

Commit Message

Gert Doering Nov. 29, 2025, 1:43 p.m. UTC
Hi,

you might have seen 2.6.17 and 2.7_rc3 releases yesterday, mentioning

    - Windows/Interactive Service: CVE-2025-13751
	fix bug where the interactive service would error-exit in
	certain error conditions instead of just logging the fact and
	continuing.  After the error-exit, OpenVPN connections will no
	longer work until the service is restarted (or the system rebooted).
	This can be triggered by any authenticated local user, and has
	thus been classified as a "local denial of service" attack.

as usual for bugs that we classify as "we do not want them to leak before
a fixed installer is out" the fix was developed & tested & reviewed in
private (Lev, Selva, Heiko).

Also, for the usual maximum transparency, I'll hereby send it to the list
- attached is the 2.7 patch, which is

commit 6088451b616be313bd47cfed8d82d47bd6e17d93
Author: Lev Stipakov <lev@openvpn.net>
Date:   Mon Nov 24 12:09:23 2025 +0200

    interactive.c: harden pipe handling against misbehaving clients


the 2.6 patch is basically the same change, but the differences are 
wide character text handling (TEXT("") vs. L"") between 2.6 and 2.7

commit 29d8eccface918703b91dde230285ca869f95090 (release/2.6)
Author: Lev Stipakov <lev@openvpn.net>
Date:   Mon Nov 24 12:09:23 2025 +0200

    interactive.c: harden pipe handling against misbehaving clients


and, finally, the 2.5 patch is a direct cherrypick from 2.6

commit 1f4b444e1407450a1071a633d73d2a63a0af06c2 (release/2.5)
Author: Lev Stipakov <lev@openvpn.net>
Date:   Mon Nov 24 12:09:23 2025 +0200

    interactive.c: harden pipe handling against misbehaving clients


Note that we do not intend to publish new 2.5.x windows installers - but
if someone is building their product on the 2.5 source tree, the fix
is in.

gert

Patch

From 6088451b616be313bd47cfed8d82d47bd6e17d93 Mon Sep 17 00:00:00 2001
From: Lev Stipakov <lev@openvpn.net>
Date: Mon, 24 Nov 2025 12:09:23 +0200
Subject: [PATCH 1/2] interactive.c: harden pipe handling against misbehaving
 clients

 - Handle ConnectNamedPipe ERROR_NO_DATA as a normal
   connect/drop race: log the drop, disconnect/reset
   that instance, and keep listening instead of letting
   a trivial local DoS stop the service.

 - Add a timed peek for startup data so a client that
   connects and sends nothing is timed out (IO_TIMEOUT)
   and rejected, instead of leaving a worker thread blocked
   forever and piling up handles.

 - Protect the accept loop from resource exhaustion: before
   spawning a worker, check the wait set and reject the client
   if adding another handle would exceed MAXIMUM_WAIT_OBJECTS;
   also skip FlushFileBuffers when no startup data was received
   to avoid hangs on silent clients.

Without these fixes, a malicious local windows user can make the OpenVPN
Interactive Service exit-on-error, thus breaking all OpenVPN connections
until the service is restarted (or the system rebooted).  Thus this has
been classified as "local denial of service" and CVE-2025-13751 has been
assigned.

CVE: 2025-13751
Change-Id: Id6a13b0c8124117bcea2926b16607ef39344015a
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpnserv/interactive.c | 60 +++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 9 deletions(-)

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 220c0427..6f04f6b5 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -208,6 +208,7 @@  ResetOverlapped(LPOVERLAPPED overlapped)
 typedef enum
 {
     peek,
+    peek_timed,
     read,
     write
 } async_op_t;
@@ -260,7 +261,7 @@  AsyncPipeOp(async_op_t op, HANDLE pipe, LPVOID buffer, DWORD size, DWORD count,
         goto out;
     }
 
-    if (op == peek)
+    if (op == peek || op == peek_timed)
     {
         PeekNamedPipe(pipe, NULL, 0, NULL, &bytes, NULL);
     }
@@ -281,6 +282,12 @@  PeekNamedPipeAsync(HANDLE pipe, DWORD count, LPHANDLE events)
     return AsyncPipeOp(peek, pipe, NULL, 0, count, events);
 }
 
+static DWORD
+PeekNamedPipeAsyncTimed(HANDLE pipe, DWORD count, LPHANDLE events)
+{
+    return AsyncPipeOp(peek_timed, pipe, NULL, 0, count, events);
+}
+
 static DWORD
 ReadPipeAsync(HANDLE pipe, LPVOID buffer, DWORD size, DWORD count, LPHANDLE events)
 {
@@ -437,11 +444,11 @@  GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
     WCHAR *data = NULL;
     DWORD bytes, read;
 
-    bytes = PeekNamedPipeAsync(pipe, 1, &exit_event);
+    bytes = PeekNamedPipeAsyncTimed(pipe, 1, &exit_event);
     if (bytes == 0)
     {
-        MsgToEventLog(M_SYSERR, L"PeekNamedPipeAsync failed");
-        ReturnLastError(pipe, L"PeekNamedPipeAsync");
+        MsgToEventLog(M_ERR, L"Timeout waiting for startup data");
+        ReturnError(pipe, ERROR_STARTUP_DATA, L"GetStartupData (timeout)", 1, &exit_event);
         goto err;
     }
 
@@ -3248,6 +3255,7 @@  RunOpenvpn(LPVOID p)
     size_t cmdline_size;
     undo_lists_t undo_lists;
     WCHAR errmsg[512] = L"";
+    BOOL flush_pipe = TRUE;
 
     SECURITY_ATTRIBUTES inheritable = { .nLength = sizeof(inheritable),
                                         .lpSecurityDescriptor = NULL,
@@ -3267,6 +3275,7 @@  RunOpenvpn(LPVOID p)
 
     if (!GetStartupData(pipe, &sud))
     {
+        flush_pipe = FALSE; /* client did not provide startup data */
         goto out;
     }
 
@@ -3562,7 +3571,10 @@  RunOpenvpn(LPVOID p)
     Undo(&undo_lists);
 
 out:
-    FlushFileBuffers(pipe);
+    if (flush_pipe)
+    {
+        FlushFileBuffers(pipe);
+    }
     DisconnectNamedPipe(pipe);
 
     free(ovpn_user);
@@ -3834,11 +3846,30 @@  ServiceStartInteractive(DWORD dwArgc, LPWSTR *lpszArgv)
 
     while (TRUE)
     {
-        if (ConnectNamedPipe(pipe, &overlapped) == FALSE && GetLastError() != ERROR_PIPE_CONNECTED
-            && GetLastError() != ERROR_IO_PENDING)
+        if (!ConnectNamedPipe(pipe, &overlapped))
         {
-            MsgToEventLog(M_SYSERR, L"Could not connect pipe");
-            break;
+            DWORD connect_error = GetLastError();
+            if (connect_error == ERROR_NO_DATA)
+            {
+                /*
+                 * Client connected and disconnected before we could process it.
+                 * Disconnect and retry instead of aborting the service.
+                 */
+                MsgToEventLog(M_ERR, L"ConnectNamedPipe returned ERROR_NO_DATA (client dropped)");
+                DisconnectNamedPipe(pipe);
+                ResetOverlapped(&overlapped);
+                continue;
+            }
+            else if (connect_error == ERROR_PIPE_CONNECTED)
+            {
+                /* No async I/O pending in this case; signal manually. */
+                SetEvent(overlapped.hEvent);
+            }
+            else if (connect_error != ERROR_IO_PENDING)
+            {
+                MsgToEventLog(M_SYSERR, L"Could not connect pipe");
+                break;
+            }
         }
 
         error = WaitForMultipleObjects(handle_count, handles, FALSE, INFINITE);
@@ -3846,6 +3877,17 @@  ServiceStartInteractive(DWORD dwArgc, LPWSTR *lpszArgv)
         {
             /* Client connected, spawn a worker thread for it */
             HANDLE next_pipe = CreateClientPipeInstance();
+
+            /* Avoid exceeding WaitForMultipleObjects MAXIMUM_WAIT_OBJECTS */
+            if (handle_count + 1 > MAXIMUM_WAIT_OBJECTS)
+            {
+                ReturnError(pipe, ERROR_CANT_WAIT, L"Too many concurrent clients", 1, &exit_event);
+                CloseHandleEx(&pipe);
+                pipe = next_pipe;
+                ResetOverlapped(&overlapped);
+                continue;
+            }
+
             HANDLE thread = CreateThread(NULL, 0, RunOpenvpn, pipe, CREATE_SUSPENDED, NULL);
             if (thread)
             {
-- 
2.51.2