From 03dbf03cf10c5ea273d909caefa3a105824a9ea0 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 30 Mar 2026 07:18:24 +0000 Subject: upstream: apply the same validity rules to usernames and hostnames set for ProxyJump/-J on the commandline as we do for destination user/host names. Specifically, they are no longer allowed to contain most characters that have special meaning for common shells. Special characters are still allowed in ProxyJump commands that are specified in the config files. This _reduces_ the chance that shell characters from a hostile -J option from ending up in a shell execution context. Don't pass untrusted stuff to the ssh commandline, it's not intended to be a security boundary. We try to make it safe where we can, but we can't make guarantees, because we can't know the parsing rules and special characters for all the shells in the world, nor can we know what the user does with this data in their ssh_config wrt percent expansion, LocalCommand, match exec, etc. While I'm in there, make ProxyJump and ProxyCommand first-match-wins between each other. reported by rabbit; ok dtucker@ Upstream: https://salsa.debian.org/ssh-team/openssh/-/blob/bookworm/debian/patches/CVE-2026-35386-1.patch Upstream: https://anongit.mindrot.org/openssh.git/commit/?id=0a0ef4515361143cad21afa072319823854c1cf6 CVE: CVE-2026-35386 Signed-off-by: Thomas Perale --- readconf.c | 124 +++++++++++++++++++++++++++++++++++++---------------- readconf.h | 4 +- ssh.c | 48 +++------------------ 3 files changed, 95 insertions(+), 81 deletions(-) diff --git a/readconf.c b/readconf.c index 2db9cb6ae..a6e8303ff 100644 --- a/readconf.c +++ b/readconf.c @@ -1483,9 +1483,6 @@ parse_char_array: case oProxyCommand: charptr = &options->proxy_command; - /* Ignore ProxyCommand if ProxyJump already specified */ - if (options->jump_host != NULL) - charptr = &options->jump_host; /* Skip below */ parse_command: if (str == NULL) { error("%.200s line %d: Missing argument.", @@ -1506,7 +1503,7 @@ parse_command: } len = strspn(str, WHITESPACE "="); /* XXX use argv? */ - if (parse_jump(str + len, options, *activep) == -1) { + if (parse_jump(str + len, options, cmdline, *activep) == -1) { error("%.200s line %d: Invalid ProxyJump \"%s\"", filename, linenum, str + len); goto out; @@ -3298,65 +3295,116 @@ parse_forward(struct Forward *fwd, const char *fwdspec, int dynamicfwd, int remo } int -parse_jump(const char *s, Options *o, int active) +ssh_valid_hostname(const char *s) { - char *orig, *sdup, *cp; - char *host = NULL, *user = NULL; - int r, ret = -1, port = -1, first; + size_t i; - active &= o->proxy_command == NULL && o->jump_host == NULL; + 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; +} - orig = sdup = xstrdup(s); +int +ssh_valid_ruser(const char *s) +{ + size_t i; - /* Remove comment and trailing whitespace */ + if (*s == '-') + return 0; + for (i = 0; s[i] != 0; i++) { + if (iscntrl((u_char)s[i])) + return 0; + 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; +} + +int +parse_jump(const char *s, Options *o, int strict, int active) +{ + char *orig = NULL, *sdup = NULL, *cp; + char *tmp_user = NULL, *tmp_host = NULL, *host = NULL, *user = NULL; + int r, ret = -1, tmp_port = -1, port = -1, first = 1; + + if (strcasecmp(s, "none") == 0) { + if (active && o->jump_host == NULL) { + o->jump_host = xstrdup("none"); + o->jump_port = 0; + } + return 0; + } + + orig = xstrdup(s); if ((cp = strchr(orig, '#')) != NULL) *cp = '\0'; rtrim(orig); - first = active; + active &= o->proxy_command == NULL && o->jump_host == NULL; + sdup = xstrdup(orig); do { - if (strcasecmp(s, "none") == 0) - break; + /* Work backwards through string */ if ((cp = strrchr(sdup, ',')) == NULL) cp = sdup; /* last */ else *cp++ = '\0'; + r = parse_ssh_uri(cp, &tmp_user, &tmp_host, &tmp_port); + if (r == -1 || (r == 1 && parse_user_host_port(cp, + &tmp_user, &tmp_host, &tmp_port) != 0)) + goto out; /* error already logged */ + if (strict) { + if (!ssh_valid_hostname(tmp_host)) { + error_f("invalid hostname \"%s\"", tmp_host); + goto out; + } + if (tmp_user != NULL && !ssh_valid_ruser(tmp_user)) { + error_f("invalid username \"%s\"", tmp_user); + goto out; + } + } if (first) { - /* First argument and configuration is active */ - r = parse_ssh_uri(cp, &user, &host, &port); - if (r == -1 || (r == 1 && - parse_user_host_port(cp, &user, &host, &port) != 0)) - goto out; - } else { - /* Subsequent argument or inactive configuration */ - r = parse_ssh_uri(cp, NULL, NULL, NULL); - if (r == -1 || (r == 1 && - parse_user_host_port(cp, NULL, NULL, NULL) != 0)) - goto out; + user = tmp_user; + host = tmp_host; + port = tmp_port; + tmp_user = tmp_host = NULL; /* transferred */ } first = 0; /* only check syntax for subsequent hosts */ + free(tmp_user); + free(tmp_host); + tmp_user = tmp_host = NULL; + tmp_port = -1; } while (cp != sdup); + /* success */ if (active) { - if (strcasecmp(s, "none") == 0) { - o->jump_host = xstrdup("none"); - o->jump_port = 0; - } else { - o->jump_user = user; - o->jump_host = host; - o->jump_port = port; - o->proxy_command = xstrdup("none"); - user = host = NULL; - if ((cp = strrchr(s, ',')) != NULL && cp != s) { - o->jump_extra = xstrdup(s); - o->jump_extra[cp - s] = '\0'; - } + o->jump_user = user; + o->jump_host = host; + o->jump_port = port; + o->proxy_command = xstrdup("none"); + user = host = NULL; /* transferred */ + if (orig != NULL && (cp = strrchr(orig, ',')) != NULL) { + o->jump_extra = xstrdup(orig); + o->jump_extra[cp - orig] = '\0'; } } ret = 0; out: free(orig); + free(sdup); + free(tmp_user); + free(tmp_host); free(user); free(host); return ret; diff --git a/readconf.h b/readconf.h index d0eb92362..591b641cb 100644 --- a/readconf.h +++ b/readconf.h @@ -243,7 +243,9 @@ int process_config_line(Options *, struct passwd *, const char *, int read_config_file(const char *, struct passwd *, const char *, const char *, Options *, int, int *); int parse_forward(struct Forward *, const char *, int, int); -int parse_jump(const char *, Options *, int); +int ssh_valid_hostname(const char *); +int ssh_valid_ruser(const char *); +int parse_jump(const char *, Options *, int, int); int parse_ssh_uri(const char *, char **, char **, int *); int default_ssh_port(void); int option_clear_or_none(const char *); diff --git a/ssh.c b/ssh.c index a0371839b..e5ec18a73 100644 --- a/ssh.c +++ b/ssh.c @@ -626,43 +626,6 @@ 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 (iscntrl((u_char)s[i])) - return 0; - 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. */ @@ -908,9 +871,9 @@ main(int ac, char **av) } if (options.proxy_command != NULL) fatal("Cannot specify -J with ProxyCommand"); - if (parse_jump(optarg, &options, 1) == -1) + if (parse_jump(optarg, &options, 1, 1) == -1) + fatal("Invalid -J argument"); - options.proxy_command = xstrdup("none"); break; case 't': if (options.request_tty == REQUEST_TTY_YES) @@ -1160,10 +1123,8 @@ main(int ac, char **av) if (!host) usage(); - if (!valid_hostname(host)) + if (!ssh_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. */ @@ -1326,7 +1287,8 @@ main(int ac, char **av) sshbin = "ssh"; /* Consistency check */ - if (options.proxy_command != NULL) + if (options.proxy_command != NULL && + strcasecmp(options.proxy_command, "none") != 0) fatal("inconsistent options: ProxyCommand+ProxyJump"); /* Never use FD passing for ProxyJump */ options.proxy_use_fdpass = 0; @@ -1449,7 +1411,7 @@ main(int ac, char **av) * Conversely, usernames from getpwnam(3) or specified as literals * via configuration (i.e. not expanded) are not subject to validation. */ - if (user_on_commandline && !valid_ruser(options.user)) + if (user_on_commandline && !ssh_valid_ruser(options.user)) fatal("remote username contains invalid characters"); /* Store it and calculate hash. */