From 8d3eae0cb6c443f5b3747aedde770fbeee7317f9 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Thu, 4 Sep 2025 00:29:09 +0000 Subject: Refuse usernames that include control characters Since OpenSSH 9.6, all usernames have been subject to validity checking. This change tightens the validity checks by refusing usernames that include control characters; these can cause surprises when supplied adversarially. This change also relaxes the validity checks in one small way: usernames supplied via the configuration file as literals are not subject to these validity checks. This allows usernames that contain arbitrary characters to be used, but only via configuration files. This is done on the basis that ssh's configuration is trusted. Pointed out by David Leadbeater, ok deraadt@ Upstream: https://salsa.debian.org/ssh-team/openssh/-/blob/bookworm/debian/patches/CVE-2025-61984.patch Upstream: https://anongit.mindrot.org/openssh.git/commit/?id=35d5917652106aede47621bb3f64044604164043 CVE: CVE-2025-61984 Signed-off-by: Thomas Perale --- ssh.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/ssh.c b/ssh.c index 48d93ddf2..9c49f98a8 100644 --- a/ssh.c +++ b/ssh.c @@ -649,6 +649,8 @@ valid_ruser(const char *s) 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 */ @@ -671,6 +673,7 @@ main(int ac, char **av) int i, r, opt, exit_status, use_syslog, direct, timeout_ms; int was_addr, config_test = 0, opt_terminated = 0, want_final_pass = 0; char *p, *cp, *line, *argv0, *logfile; + int user_on_commandline = 0, user_was_default = 0; char cname[NI_MAXHOST], thishost[NI_MAXHOST]; struct stat st; struct passwd *pw; @@ -1016,8 +1019,10 @@ main(int ac, char **av) } break; case 'l': - if (options.user == NULL) + if (options.user == NULL) { options.user = optarg; + user_on_commandline = 1; + } break; case 'L': @@ -1120,6 +1125,7 @@ main(int ac, char **av) if (options.user == NULL) { options.user = tuser; tuser = NULL; + user_on_commandline = 1; } free(tuser); if (options.port == -1 && tport != -1) @@ -1134,6 +1140,7 @@ main(int ac, char **av) if (options.user == NULL) { options.user = p; p = NULL; + user_on_commandline = 1; } *cp++ = '\0'; host = xstrdup(cp); @@ -1288,8 +1295,10 @@ main(int ac, char **av) if (fill_default_options(&options) != 0) cleanup_exit(255); - if (options.user == NULL) + if (options.user == NULL) { + user_was_default = 1; options.user = xstrdup(pw->pw_name); + } /* * If ProxyJump option specified, then construct a ProxyCommand now. @@ -1430,11 +1439,22 @@ main(int ac, char **av) options.host_key_alias : options.host_arg); cinfo->host_arg = xstrdup(options.host_arg); cinfo->remhost = xstrdup(host); - cinfo->remuser = xstrdup(options.user); cinfo->homedir = xstrdup(pw->pw_dir); cinfo->locuser = xstrdup(pw->pw_name); cinfo->jmphost = xstrdup(options.jump_host == NULL ? "" : options.jump_host); + + /* + * Usernames specified on the commandline must be validated. + * 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)) + fatal("remote username contains invalid characters"); + + /* Store it and calculate hash. */ + cinfo->remuser = xstrdup(options.user); + cinfo->conn_hash_hex = ssh_connection_hash(cinfo->thishost, cinfo->remhost, cinfo->portstr, cinfo->remuser, cinfo->jmphost);