diff mbox series

[Openvpn-devel,v1] win32: quote arguments that cmd.exe would reinterpret

Message ID 20260902121416.7047-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v1] win32: quote arguments that cmd.exe would reinterpret |

Commit Message

Gert Doering Sept. 2, 2026, 12:14 p.m. UTC 
  From: Lev Stipakov <lev@openvpn.net>

wide_cmd_line() only quoted arguments containing a space, so & | < > ^ %
( ) and ! were passed unquoted. CreateProcess() runs .bat and .cmd files
through cmd.exe, which parses the command line again, so a certificate
subject passed to --tls-verify could start a second command (CERT/CC
VU#123335).

Quote on those characters too. Double quotes are already replaced with
'_', so nothing else needs escaping. The delimiters , ; and = are left
out: cmd.exe uses them to separate %1..%9 but cannot run anything with
them, and quoting them would change what existing scripts receive.

GitHub: OpenVPN/openvpn-private-issues#159
Reported-By: Clouditera Security; Z.ai Security; NSFOCUS <security@clouditera.com>
CVE: 2026-84256
Change-Id: I68429deb39c0bae335d46a4170973a14c644ccf9
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Heiko Hund <heiko@openvpn.net>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1891
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1891
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Heiko Hund <heiko@openvpn.net>

Comments

Gert Doering Sept. 2, 2026, 3:35 p.m. UTC | #1 
Thanks for patch and review... weird corner cases of corner cases, but
a possible security issue nonetheless.

I have tested that this compiles & and the unit tests (next patch) work
via GHA builds.  I haven't tested this in the wild, but I'm sure Lev has.

Your patch has been applied to the master, release/2.7 and release/2.6 branch.

commit cf08504bbbacdb0c3d94c7a0358a6ec06c553cdd (master)
commit 26653e7dda48dacf8f886dd2c85476f9786cab0e (release/2.7)
commit 3c3ef5b7d7945502a2d7ee4fab1d7f928b392d8c (release/2.6)
Author: Lev Stipakov
Date:   Wed Sep 2 14:14:09 2026 +0200

     win32: quote arguments that cmd.exe would reinterpret

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Heiko Hund <heiko@openvpn.net>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1891
     Message-Id: <20260902121416.7047-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg38897.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index 44b010d..d527160 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -936,6 +936,22 @@ 
     }
 }
 
+/* special to cmd.exe, which CreateProcess() uses to run .bat/.cmd (VU#123335) */
+#define CMD_QUOTE_TRIGGERS " &|<>^%()!"
+
+static bool
+argv_element_needs_quotes(const char *str)
+{
+    for (const char *c = str; *c != '\0'; ++c)
+    {
+        if (strchr(CMD_QUOTE_TRIGGERS, *c) != NULL)
+        {
+            return true;
+        }
+    }
+    return false;
+}
+
 static WCHAR *
 wide_cmd_line(const struct argv *a, struct gc_arena *gc)
 {
@@ -974,13 +990,13 @@ 
         {
             buf_printf(&buf, " ");
         }
-        if (string_class(work, CC_ANY, CC_SPACE))
+        if (argv_element_needs_quotes(work))
         {
-            buf_printf(&buf, "%s", work);
+            buf_printf(&buf, "\"%s\"", work);
         }
         else
         {
-            buf_printf(&buf, "\"%s\"", work);
+            buf_printf(&buf, "%s", work);
         }
     }