Skip to content

Commit 6c3afe5

Browse files
lstipakovcron2
authored andcommitted
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 <[email protected]> Acked-by: Gert Doering <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1198 Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg33071.html Signed-off-by: Gert Doering <[email protected]>
1 parent cabbf49 commit 6c3afe5

File tree

3 files changed

+59
-0
lines changed

3 files changed

+59
-0
lines changed

src/openvpn/domain_helper.h

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* OpenVPN -- An application to securely tunnel IP networks
3+
* over a single UDP port, with support for SSL/TLS-based
4+
* session authentication and key exchange,
5+
* packet encryption, packet authentication, and
6+
* packet compression.
7+
*
8+
* Copyright (C) 2025 Lev Stipakov <[email protected]>
9+
*
10+
* This program is free software; you can redistribute it and/or modify
11+
* it under the terms of the GNU General Public License version 2
12+
* as published by the Free Software Foundation.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU General Public License along
20+
* with this program; if not, write to the Free Software Foundation, Inc.,
21+
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
22+
*/
23+
24+
static inline bool
25+
is_allowed_domain_ascii(unsigned char c)
26+
{
27+
return (c >= 'A' && c <= 'Z')
28+
|| (c >= 'a' && c <= 'z')
29+
|| (c >= '0' && c <= '9')
30+
|| c == '.' || c == '-' || c == '_' || c >= 0x80;
31+
}
32+
33+
static inline bool
34+
validate_domain(const char *domain)
35+
{
36+
for (const char *ch = domain; *ch; ++ch)
37+
{
38+
if (!is_allowed_domain_ascii((unsigned char)*ch))
39+
{
40+
return false;
41+
}
42+
}
43+
44+
return true;
45+
}

src/openvpn/tun.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "win32.h"
4747
#include "block_dns.h"
4848
#include "networking.h"
49+
#include "domain_helper.h"
4950

5051
#include "memdbg.h"
5152

@@ -390,6 +391,12 @@ do_dns_domain_pwsh(bool add, const struct tuntap *tt)
390391
return;
391392
}
392393

394+
if (add && !validate_domain(tt->options.domain))
395+
{
396+
msg(M_WARN, "Failed to set DNS domain '%s' because it contains invalid characters", tt->options.domain);
397+
return;
398+
}
399+
393400
struct argv argv = argv_new();
394401
argv_printf(&argv,
395402
"%s%s -NoProfile -NonInteractive -Command Set-DnsClient -InterfaceIndex %lu -ConnectionSpecificSuffix '%s'",

src/openvpnserv/interactive.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "validate.h"
4141
#include "block_dns.h"
4242
#include "ring_buffer.h"
43+
#include "domain_helper.h"
4344

4445
#define IO_TIMEOUT 2000 /*ms*/
4546

@@ -1216,6 +1217,12 @@ SetDNSDomain(const wchar_t *if_name, const char *domain, undo_lists_t *lists)
12161217
{
12171218
NET_IFINDEX if_index;
12181219

1220+
if (!validate_domain(domain))
1221+
{
1222+
MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Failed to set DNS domain '%hs' because it contains invalid characters"), domain);
1223+
return ERROR_INVALID_DATA;
1224+
}
1225+
12191226
DWORD err = ConvertInterfaceNameToIndex(if_name, &if_index);
12201227
if (err != ERROR_SUCCESS)
12211228
{

0 commit comments

Comments
 (0)