Browse code

Validate DNS domain name before powershell invocation

Starting from commit

d383d6e ("win: replace wmic invocation with powershell")

we pass --dhcp-option DOMAIN value to a powershell command
to set DNS domain. Without validation this opens the door
to a command injection atack.

This only allows domain names with characters:

[A-Za-z0-9.-_\x80-\0xff]

Change-Id: I7a57d7b4e84aa2b9c9e71e30520ed468b0e3c278
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1198
Message-Id: <20250918173447.32466-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33071.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Lev Stipakov authored on 2025/09/19 02:34:40
Showing 3 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,45 @@
0
+/*
1
+ *  OpenVPN -- An application to securely tunnel IP networks
2
+ *             over a single UDP port, with support for SSL/TLS-based
3
+ *             session authentication and key exchange,
4
+ *             packet encryption, packet authentication, and
5
+ *             packet compression.
6
+ *
7
+ *  Copyright (C) 2025 Lev Stipakov <lev@openvpn.net>
8
+ *
9
+ *  This program is free software; you can redistribute it and/or modify
10
+ *  it under the terms of the GNU General Public License version 2
11
+ *  as published by the Free Software Foundation.
12
+ *
13
+ *  This program is distributed in the hope that it will be useful,
14
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
15
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
16
+ *  GNU General Public License for more details.
17
+ *
18
+ *  You should have received a copy of the GNU General Public License along
19
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
20
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
21
+ */
22
+
23
+static inline bool
24
+is_allowed_domain_ascii(unsigned char c)
25
+{
26
+    return (c >= 'A' && c <= 'Z')
27
+           || (c >= 'a' && c <= 'z')
28
+           || (c >= '0' && c <= '9')
29
+           || c == '.' || c == '-' || c == '_' || c >= 0x80;
30
+}
31
+
32
+static inline bool
33
+validate_domain(const char *domain)
34
+{
35
+    for (const char *ch = domain; *ch; ++ch)
36
+    {
37
+        if (!is_allowed_domain_ascii((unsigned char)*ch))
38
+        {
39
+            return false;
40
+        }
41
+    }
42
+
43
+    return true;
44
+}
... ...
@@ -46,6 +46,7 @@
46 46
 #include "win32.h"
47 47
 #include "block_dns.h"
48 48
 #include "networking.h"
49
+#include "domain_helper.h"
49 50
 
50 51
 #include "memdbg.h"
51 52
 
... ...
@@ -390,6 +391,12 @@ do_dns_domain_pwsh(bool add, const struct tuntap *tt)
390 390
         return;
391 391
     }
392 392
 
393
+    if (add && !validate_domain(tt->options.domain))
394
+    {
395
+        msg(M_WARN, "Failed to set DNS domain '%s' because it contains invalid characters", tt->options.domain);
396
+        return;
397
+    }
398
+
393 399
     struct argv argv = argv_new();
394 400
     argv_printf(&argv,
395 401
                 "%s%s -NoProfile -NonInteractive -Command Set-DnsClient -InterfaceIndex %lu -ConnectionSpecificSuffix '%s'",
... ...
@@ -40,6 +40,7 @@
40 40
 #include "validate.h"
41 41
 #include "block_dns.h"
42 42
 #include "ring_buffer.h"
43
+#include "domain_helper.h"
43 44
 
44 45
 #define IO_TIMEOUT  2000 /*ms*/
45 46
 
... ...
@@ -1216,6 +1217,12 @@ SetDNSDomain(const wchar_t *if_name, const char *domain, undo_lists_t *lists)
1216 1216
 {
1217 1217
     NET_IFINDEX if_index;
1218 1218
 
1219
+    if (!validate_domain(domain))
1220
+    {
1221
+        MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Failed to set DNS domain '%hs' because it contains invalid characters"), domain);
1222
+        return ERROR_INVALID_DATA;
1223
+    }
1224
+
1219 1225
     DWORD err  = ConvertInterfaceNameToIndex(if_name, &if_index);
1220 1226
     if (err != ERROR_SUCCESS)
1221 1227
     {