Skip to content

Commit 7ef3787

Browse files
committed
upstream: ban user/hostnames with most shell metacharacters
This makes ssh(1) refuse user or host names provided on the commandline that contain most shell metacharacters. Some programs that invoke ssh(1) using untrusted data do not filter metacharacters in arguments they supply. This could create interactions with user-specified ProxyCommand and other directives that allow shell injection attacks to occur. It's a mistake to invoke ssh(1) with arbitrary untrusted arguments, but getting this stuff right can be tricky, so this should prevent most obvious ways of creating risky situations. It however is not and cannot be perfect: ssh(1) has no practical way of interpreting what shell quoting rules are in use and how they interact with the user's specified ProxyCommand. To allow configurations that use strange user or hostnames to continue to work, this strictness is applied only to names coming from the commandline. Names specified using User or Hostname directives in ssh_config(5) are not affected. feedback/ok millert@ markus@ dtucker@ deraadt@ OpenBSD-Commit-ID: 3b487348b5964f3e77b6b4d3da4c3b439e94b2d9
1 parent 0cb50ee commit 7ef3787

File tree

1 file changed

+40
-1
lines changed

1 file changed

+40
-1
lines changed

ssh.c

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: ssh.c,v 1.598 2023/10/12 02:48:43 djm Exp $ */
1+
/* $OpenBSD: ssh.c,v 1.599 2023/12/18 14:47:44 djm Exp $ */
22
/*
33
* Author: Tatu Ylonen <[email protected]>
44
* Copyright (c) 1995 Tatu Ylonen <[email protected]>, Espoo, Finland
@@ -626,6 +626,41 @@ ssh_conn_info_free(struct ssh_conn_info *cinfo)
626626
free(cinfo);
627627
}
628628

629+
static int
630+
valid_hostname(const char *s)
631+
{
632+
size_t i;
633+
634+
if (*s == '-')
635+
return 0;
636+
for (i = 0; s[i] != 0; i++) {
637+
if (strchr("'`\"$\\;&<>|(){}", s[i]) != NULL ||
638+
isspace((u_char)s[i]) || iscntrl((u_char)s[i]))
639+
return 0;
640+
}
641+
return 1;
642+
}
643+
644+
static int
645+
valid_ruser(const char *s)
646+
{
647+
size_t i;
648+
649+
if (*s == '-')
650+
return 0;
651+
for (i = 0; s[i] != 0; i++) {
652+
if (strchr("'`\";&<>|(){}", s[i]) != NULL)
653+
return 0;
654+
/* Disallow '-' after whitespace */
655+
if (isspace((u_char)s[i]) && s[i + 1] == '-')
656+
return 0;
657+
/* Disallow \ in last position */
658+
if (s[i] == '\\' && s[i + 1] == '\0')
659+
return 0;
660+
}
661+
return 1;
662+
}
663+
629664
/*
630665
* Main program for the ssh client.
631666
*/
@@ -1118,6 +1153,10 @@ main(int ac, char **av)
11181153
if (!host)
11191154
usage();
11201155

1156+
if (!valid_hostname(host))
1157+
fatal("hostname contains invalid characters");
1158+
if (options.user != NULL && !valid_ruser(options.user))
1159+
fatal("remote username contains invalid characters");
11211160
options.host_arg = xstrdup(host);
11221161

11231162
/* Initialize the command to execute on remote host. */

0 commit comments

Comments
 (0)