CVE-2023-51385: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: In ssh in OpenSSH before 9.6, OS command injection might occur if a user name or host name has shell metacharacters, and this name is referenced by an expansion token in certain situations. For example, an untrusted Git repository can have a submodule with shell metacharacters in a user name or host name.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 6.5
Published: 2023-12-18T19:15Z
Repository: https://github.com/openssh/openssh-portable
Language: C
Files changed: ssh.c
Commit date: 2023-12-19 01:53:40 +1100
Commit message: 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

Diff: diff --git a/ssh.c b/ssh.c
index 35c48e62d1..48d93ddf2a 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1,6 +1,6 @@
-/* $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
  *                    All rights reserved
  * Ssh client program.  This program can be used to log into a remote machine.
@@ -624,10 +624,45 @@ ssh_conn_info_free(struct ssh_conn_info *cinfo)
 	free(cinfo->locuser);
 	free(cinfo->jmphost);
 	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.
  */
 int
 main(int ac, char **av)
@@ -1116,10 +1151,14 @@ main(int ac, char **av)
 
 	/* Check that we got a host name. */
 	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. */
 	if ((command = sshbuf_new()) == NULL)
 		fatal("sshbuf_new failed");

Vulnerable code: /* $OpenBSD: ssh.c,v 1.598 2023/10/12 02:48:43 djm Exp $ */
Fixed code: /* $OpenBSD: ssh.c,v 1.599 2023/12/18 14:47:44 djm Exp $ */
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;
}

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