Skip to content

Commit

Permalink
upstream: ban user/hostnames with most shell metacharacters
Browse files Browse the repository at this point in the history
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
  • Loading branch information
djmdjm committed Dec 18, 2023
1 parent 0cb50ee commit 7ef3787
Showing 1 changed file with 40 additions and 1 deletion.
41 changes: 40 additions & 1 deletion ssh.c
@@ -1,4 +1,4 @@
/* $OpenBSD: ssh.c,v 1.598 2023/10/12 02:48:43 djm Exp $ */
/* $OpenBSD: ssh.c,v 1.599 2023/12/18 14:47:44 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
Expand Down Expand Up @@ -626,6 +626,41 @@ ssh_conn_info_free(struct ssh_conn_info *cinfo)
free(cinfo);
}

static int
valid_hostname(const char *s)
{
size_t i;

if (*s == '-')
return 0;
for (i = 0; s[i] != 0; i++) {
if (strchr("'`\"$\\;&<>|(){}", s[i]) != NULL ||
isspace((u_char)s[i]) || iscntrl((u_char)s[i]))
return 0;
}
return 1;
}

static int
valid_ruser(const char *s)
{
size_t i;

if (*s == '-')
return 0;
for (i = 0; s[i] != 0; i++) {
if (strchr("'`\";&<>|(){}", s[i]) != NULL)
return 0;
/* Disallow '-' after whitespace */
if (isspace((u_char)s[i]) && s[i + 1] == '-')
return 0;
/* Disallow \ in last position */
if (s[i] == '\\' && s[i + 1] == '\0')
return 0;
}
return 1;
}

/*
* Main program for the ssh client.
*/
Expand Down Expand Up @@ -1118,6 +1153,10 @@ main(int ac, char **av)
if (!host)
usage();

if (!valid_hostname(host))
fatal("hostname contains invalid characters");
if (options.user != NULL && !valid_ruser(options.user))
fatal("remote username contains invalid characters");
options.host_arg = xstrdup(host);

/* Initialize the command to execute on remote host. */
Expand Down

0 comments on commit 7ef3787

Please sign in to comment.