[S] Change in openvpn[master]: win32: quote arguments that cmd.exe would reinterpret

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1891?usp=email

to review the following change.


Change subject: win32: quote arguments that cmd.exe would reinterpret
......................................................................

win32: quote arguments that cmd.exe would reinterpret

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 <[email protected]>
CVE: 2026-84256
Change-Id: I68429deb39c0bae335d46a4170973a14c644ccf9
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/win32.c
1 file changed, 19 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/91/1891/1

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);
         }
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1891?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I68429deb39c0bae335d46a4170973a14c644ccf9
Gerrit-Change-Number: 1891
Gerrit-PatchSet: 1
Gerrit-Owner: cron2 <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel