[openssh-commits] [openssh] 01/07: upstream: prepare readconf.c for fuzzing; remove fatal calls and

git+noreply at mindrot.org git+noreply at mindrot.org
Mon Dec 21 10:52:35 AEDT 2020


This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit 43026da035cd266db37df1f723d5575056150744
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Thu Dec 17 23:10:27 2020 +0000

    upstream: prepare readconf.c for fuzzing; remove fatal calls and
    
    fix some (one-off) memory leaks; ok markus@
    
    OpenBSD-Commit-ID: 91c6aec57b0e7aae9190de188e9fe8933aad5ec5
---
 readconf.c    | 559 ++++++++++++++++++++++++++++++++++++++++++----------------
 readconf.h    |   5 +-
 ssh-keysign.c |   4 +-
 ssh.c         |   5 +-
 4 files changed, 412 insertions(+), 161 deletions(-)

diff --git a/readconf.c b/readconf.c
index d60eeacf..c8ba5ddb 100644
--- a/readconf.c
+++ b/readconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.c,v 1.343 2020/11/30 05:36:39 dtucker Exp $ */
+/* $OpenBSD: readconf.c,v 1.344 2020/12/17 23:10:27 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo at cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -315,14 +315,20 @@ static struct {
 	{ NULL, oBadOption }
 };
 
-static char *kex_default_pk_alg_filtered;
 
 const char *
 kex_default_pk_alg(void)
 {
-	if (kex_default_pk_alg_filtered == NULL)
-		fatal("kex_default_pk_alg not initialized.");
-	return kex_default_pk_alg_filtered;
+	static char *pkalgs;
+
+	if (pkalgs == NULL) {
+		char *all_key;
+
+		all_key = sshkey_alg_list(0, 0, 1, ',');
+		pkalgs = match_filter_allowlist(KEX_DEFAULT_PK_ALG, all_key);
+		free(all_key);
+	}
+	return pkalgs;
 }
 
 char *
@@ -873,8 +879,10 @@ parse_multistate_value(const char *arg, const char *filename, int linenum,
 {
 	int i;
 
-	if (!arg || *arg == '\0')
-		fatal("%s line %d: missing argument.", filename, linenum);
+	if (!arg || *arg == '\0') {
+		error("%s line %d: missing argument.", filename, linenum);
+		return -1;
+	}
 	for (i = 0; multistate_ptr[i].key != NULL; i++) {
 		if (strcasecmp(arg, multistate_ptr[i].key) == 0)
 			return multistate_ptr[i].value;
@@ -959,14 +967,18 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host,
 		intptr = &options->connection_timeout;
 parse_time:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%s line %d: missing time value.",
+		if (!arg || *arg == '\0') {
+			error("%s line %d: missing time value.",
 			    filename, linenum);
+			return -1;
+		}
 		if (strcmp(arg, "none") == 0)
 			value = -1;
-		else if ((value = convtime(arg)) == -1)
-			fatal("%s line %d: invalid time value.",
+		else if ((value = convtime(arg)) == -1) {
+			error("%s line %d: invalid time value.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*activep && *intptr == -1)
 			*intptr = value;
 		break;
@@ -975,9 +987,11 @@ parse_time:
 		intptr = &options->forward_agent;
 
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%s line %d: missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%s line %d: missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 
 		value = -1;
 		multistate_ptr = multistate_flag;
@@ -1007,8 +1021,9 @@ parse_time:
 		arg = strdelim(&s);
 		if ((value = parse_multistate_value(arg, filename, linenum,
 		     multistate_ptr)) == -1) {
-			fatal("%s line %d: unsupported option \"%s\".",
+			error("%s line %d: unsupported option \"%s\".",
 			    filename, linenum, arg);
+			return -1;
 		}
 		if (*activep && *intptr == -1)
 			*intptr = value;
@@ -1099,18 +1114,24 @@ parse_time:
 
 	case oRekeyLimit:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename,
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.", filename,
 			    linenum);
+			return -1;
+		}
 		if (strcmp(arg, "default") == 0) {
 			val64 = 0;
 		} else {
-			if (scan_scaled(arg, &val64) == -1)
-				fatal("%.200s line %d: Bad number '%s': %s",
+			if (scan_scaled(arg, &val64) == -1) {
+				error("%.200s line %d: Bad number '%s': %s",
 				    filename, linenum, arg, strerror(errno));
-			if (val64 != 0 && val64 < 16)
-				fatal("%.200s line %d: RekeyLimit too small",
+				return -1;
+			}
+			if (val64 != 0 && val64 < 16) {
+				error("%.200s line %d: RekeyLimit too small",
 				    filename, linenum);
+				return -1;
+			}
 		}
 		if (*activep && options->rekey_limit == -1)
 			options->rekey_limit = val64;
@@ -1126,13 +1147,19 @@ parse_time:
 
 	case oIdentityFile:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		if (*activep) {
 			intptr = &options->num_identity_files;
-			if (*intptr >= SSH_MAX_IDENTITY_FILES)
-				fatal("%.200s line %d: Too many identity files specified (max %d).",
-				    filename, linenum, SSH_MAX_IDENTITY_FILES);
+			if (*intptr >= SSH_MAX_IDENTITY_FILES) {
+				error("%.200s line %d: Too many identity files "
+				    "specified (max %d).", filename, linenum,
+				    SSH_MAX_IDENTITY_FILES);
+				return -1;
+			}
 			add_identity_file(options, NULL,
 			    arg, flags & SSHCONF_USERCONF);
 		}
@@ -1140,16 +1167,19 @@ parse_time:
 
 	case oCertificateFile:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*activep) {
 			intptr = &options->num_certificate_files;
 			if (*intptr >= SSH_MAX_CERTIFICATE_FILES) {
-				fatal("%.200s line %d: Too many certificate "
+				error("%.200s line %d: Too many certificate "
 				    "files specified (max %d).",
 				    filename, linenum,
 				    SSH_MAX_CERTIFICATE_FILES);
+				return -1;
 			}
 			add_certificate_file(options, arg,
 			    flags & SSHCONF_USERCONF);
@@ -1164,9 +1194,11 @@ parse_time:
 		charptr = &options->user;
 parse_string:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*activep && *charptr == NULL)
 			*charptr = xstrdup(arg);
 		break;
@@ -1178,10 +1210,11 @@ parse_string:
 parse_char_array:
 		if (*activep && *uintptr == 0) {
 			while ((arg = strdelim(&s)) != NULL && *arg != '\0') {
-				if ((*uintptr) >= max_entries)
-					fatal("%s line %d: "
-					    "too many known hosts files.",
-					    filename, linenum);
+				if ((*uintptr) >= max_entries) {
+					error("%s line %d: too many known "
+					    "hosts files.", filename, linenum);
+					return -1;
+				}
 				cpptr[(*uintptr)++] = xstrdup(arg);
 			}
 		}
@@ -1227,8 +1260,11 @@ parse_char_array:
 		if (options->jump_host != NULL)
 			charptr = &options->jump_host; /* Skip below */
 parse_command:
-		if (s == NULL)
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (s == NULL) {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		len = strspn(s, WHITESPACE "=");
 		if (*activep && *charptr == NULL)
 			*charptr = xstrdup(s + len);
@@ -1236,25 +1272,31 @@ parse_command:
 
 	case oProxyJump:
 		if (s == NULL) {
-			fatal("%.200s line %d: Missing argument.",
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
 		}
 		len = strspn(s, WHITESPACE "=");
 		if (parse_jump(s + len, options, *activep) == -1) {
-			fatal("%.200s line %d: Invalid ProxyJump \"%s\"",
+			error("%.200s line %d: Invalid ProxyJump \"%s\"",
 			    filename, linenum, s + len);
+			return -1;
 		}
 		return 0;
 
 	case oPort:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 		value = a2port(arg);
-		if (value <= 0)
-			fatal("%.200s line %d: Bad port '%s'.",
+		if (value <= 0) {
+			error("%.200s line %d: Bad port '%s'.",
 			    filename, linenum, arg);
+			return -1;
+		}
 		if (*activep && options->port == -1)
 			options->port = value;
 		break;
@@ -1263,47 +1305,63 @@ parse_command:
 		intptr = &options->connection_attempts;
 parse_int:
 		arg = strdelim(&s);
-		if ((errstr = atoi_err(arg, &value)) != NULL)
-			fatal("%s line %d: integer value %s.",
+		if ((errstr = atoi_err(arg, &value)) != NULL) {
+			error("%s line %d: integer value %s.",
 			    filename, linenum, errstr);
+			return -1;
+		}
 		if (*activep && *intptr == -1)
 			*intptr = value;
 		break;
 
 	case oCiphers:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		if (*arg != '-' &&
-		    !ciphers_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg))
-			fatal("%.200s line %d: Bad SSH2 cipher spec '%s'.",
+		    !ciphers_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg)){
+			error("%.200s line %d: Bad SSH2 cipher spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*activep && options->ciphers == NULL)
 			options->ciphers = xstrdup(arg);
 		break;
 
 	case oMacs:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		if (*arg != '-' &&
-		    !mac_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg))
-			fatal("%.200s line %d: Bad SSH2 MAC spec '%s'.",
+		    !mac_valid(*arg == '+' || *arg == '^' ? arg + 1 : arg)) {
+			error("%.200s line %d: Bad SSH2 MAC spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*activep && options->macs == NULL)
 			options->macs = xstrdup(arg);
 		break;
 
 	case oKexAlgorithms:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*arg != '-' &&
 		    !kex_names_valid(*arg == '+' || *arg == '^' ?
-		    arg + 1 : arg))
-			fatal("%.200s line %d: Bad SSH2 KexAlgorithms '%s'.",
+		    arg + 1 : arg)) {
+			error("%.200s line %d: Bad SSH2 KexAlgorithms '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*activep && options->kex_algorithms == NULL)
 			options->kex_algorithms = xstrdup(arg);
 		break;
@@ -1312,14 +1370,18 @@ parse_int:
 		charptr = &options->hostkeyalgorithms;
 parse_keytypes:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*arg != '-' &&
 		    !sshkey_names_valid2(*arg == '+' || *arg == '^' ?
-		    arg + 1 : arg, 1))
-			fatal("%s line %d: Bad key types '%s'.",
-				filename, linenum, arg ? arg : "<NONE>");
+		    arg + 1 : arg, 1)) {
+			error("%s line %d: Bad key types '%s'.",
+			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*activep && *charptr == NULL)
 			*charptr = xstrdup(arg);
 		break;
@@ -1332,9 +1394,11 @@ parse_keytypes:
 		log_level_ptr = &options->log_level;
 		arg = strdelim(&s);
 		value = log_level_number(arg);
-		if (value == SYSLOG_LEVEL_NOT_SET)
-			fatal("%.200s line %d: unsupported log level '%s'",
+		if (value == SYSLOG_LEVEL_NOT_SET) {
+			error("%.200s line %d: unsupported log level '%s'",
 			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*activep && *log_level_ptr == SYSLOG_LEVEL_NOT_SET)
 			*log_level_ptr = (LogLevel) value;
 		break;
@@ -1343,9 +1407,11 @@ parse_keytypes:
 		log_facility_ptr = &options->log_facility;
 		arg = strdelim(&s);
 		value = log_facility_number(arg);
-		if (value == SYSLOG_FACILITY_NOT_SET)
-			fatal("%.200s line %d: unsupported log facility '%s'",
+		if (value == SYSLOG_FACILITY_NOT_SET) {
+			error("%.200s line %d: unsupported log facility '%s'",
 			    filename, linenum, arg ? arg : "<NONE>");
+			return -1;
+		}
 		if (*log_facility_ptr == -1)
 			*log_facility_ptr = (SyslogFacility) value;
 		break;
@@ -1366,9 +1432,11 @@ parse_keytypes:
 	case oRemoteForward:
 	case oDynamicForward:
 		arg = strdelim(&s);
-		if (arg == NULL || *arg == '\0')
-			fatal("%.200s line %d: Missing port argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
 
 		remotefwd = (opcode == oRemoteForward);
 		dynamicfwd = (opcode == oDynamicForward);
@@ -1378,9 +1446,11 @@ parse_keytypes:
 			if (arg2 == NULL || *arg2 == '\0') {
 				if (remotefwd)
 					dynamicfwd = 1;
-				else
-					fatal("%.200s line %d: Missing target "
+				else {
+					error("%.200s line %d: Missing target "
 					    "argument.", filename, linenum);
+					return -1;
+				}
 			} else {
 				/* construct a string for parse_forward */
 				snprintf(fwdarg, sizeof(fwdarg), "%s:%s", arg,
@@ -1390,9 +1460,11 @@ parse_keytypes:
 		if (dynamicfwd)
 			strlcpy(fwdarg, arg, sizeof(fwdarg));
 
-		if (parse_forward(&fwd, fwdarg, dynamicfwd, remotefwd) == 0)
-			fatal("%.200s line %d: Bad forwarding specification.",
+		if (parse_forward(&fwd, fwdarg, dynamicfwd, remotefwd) == 0) {
+			error("%.200s line %d: Bad forwarding specification.",
 			    filename, linenum);
+			return -1;
+		}
 
 		if (*activep) {
 			if (remotefwd) {
@@ -1408,9 +1480,11 @@ parse_keytypes:
 		goto parse_flag;
 
 	case oHost:
-		if (cmdline)
-			fatal("Host directive not supported as a command-line "
+		if (cmdline) {
+			error("Host directive not supported as a command-line "
 			    "option");
+			return -1;
+		}
 		*activep = 0;
 		arg2 = NULL;
 		while ((arg = strdelim(&s)) != NULL && *arg != '\0') {
@@ -1440,23 +1514,30 @@ parse_keytypes:
 		return 0;
 
 	case oMatch:
-		if (cmdline)
-			fatal("Host directive not supported as a command-line "
+		if (cmdline) {
+			error("Host directive not supported as a command-line "
 			    "option");
+			return -1;
+		}
 		value = match_cfg_line(options, &s, pw, host, original_host,
 		    flags & SSHCONF_FINAL, want_final_pass,
 		    filename, linenum);
-		if (value < 0)
-			fatal("%.200s line %d: Bad Match condition", filename,
+		if (value < 0) {
+			error("%.200s line %d: Bad Match condition", filename,
 			    linenum);
+			return -1;
+		}
 		*activep = (flags & SSHCONF_NEVERMATCH) ? 0 : value;
 		break;
 
 	case oEscapeChar:
 		intptr = &options->escape_char;
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		if (strcmp(arg, "none") == 0)
 			value = SSH_ESCAPECHAR_NONE;
 		else if (arg[1] == '\0')
@@ -1465,9 +1546,9 @@ parse_keytypes:
 		    (u_char) arg[1] >= 64 && (u_char) arg[1] < 128)
 			value = (u_char) arg[1] & 31;
 		else {
-			value = 0;	/* Avoid compiler warning. */
-			fatal("%.200s line %d: Bad escape character.",
+			error("%.200s line %d: Bad escape character.",
 			    filename, linenum);
+			return -1;
 		}
 		if (*activep && *intptr == -1)
 			*intptr = value;
@@ -1496,9 +1577,11 @@ parse_keytypes:
 
 	case oSendEnv:
 		while ((arg = strdelim(&s)) != NULL && *arg != '\0') {
-			if (strchr(arg, '=') != NULL)
-				fatal("%s line %d: Invalid environment name.",
+			if (strchr(arg, '=') != NULL) {
+				error("%s line %d: Invalid environment name.",
 				    filename, linenum);
+				return -1;
+			}
 			if (!*activep)
 				continue;
 			if (*arg == '-') {
@@ -1507,9 +1590,11 @@ parse_keytypes:
 				continue;
 			} else {
 				/* Adding an env var */
-				if (options->num_send_env >= INT_MAX)
-					fatal("%s line %d: too many send env.",
+				if (options->num_send_env >= INT_MAX) {
+					error("%s line %d: too many send env.",
 					    filename, linenum);
+					return -1;
+				}
 				options->send_env = xrecallocarray(
 				    options->send_env, options->num_send_env,
 				    options->num_send_env + 1,
@@ -1523,15 +1608,19 @@ parse_keytypes:
 	case oSetEnv:
 		value = options->num_setenv;
 		while ((arg = strdelimw(&s)) != NULL && *arg != '\0') {
-			if (strchr(arg, '=') == NULL)
-				fatal("%s line %d: Invalid SetEnv.",
+			if (strchr(arg, '=') == NULL) {
+				error("%s line %d: Invalid SetEnv.",
 				    filename, linenum);
+				return -1;
+			}
 			if (!*activep || value != 0)
 				continue;
 			/* Adding a setenv var */
-			if (options->num_setenv >= INT_MAX)
-				fatal("%s line %d: too many SetEnv.",
+			if (options->num_setenv >= INT_MAX) {
+				error("%s line %d: too many SetEnv.",
 				    filename, linenum);
+				return -1;
+			}
 			options->setenv = xrecallocarray(
 			    options->setenv, options->num_setenv,
 			    options->num_setenv + 1, sizeof(*options->setenv));
@@ -1552,9 +1641,11 @@ parse_keytypes:
 		/* no/false/yes/true, or a time spec */
 		intptr = &options->control_persist;
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing ControlPersist"
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing ControlPersist"
 			    " argument.", filename, linenum);
+			return -1;
+		}
 		value = 0;
 		value2 = 0;	/* timeout */
 		if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0)
@@ -1563,9 +1654,11 @@ parse_keytypes:
 			value = 1;
 		else if ((value2 = convtime(arg)) >= 0)
 			value = 1;
-		else
-			fatal("%.200s line %d: Bad ControlPersist argument.",
+		else {
+			error("%.200s line %d: Bad ControlPersist argument.",
 			    filename, linenum);
+			return -1;
+		}
 		if (*activep && *intptr == -1) {
 			*intptr = value;
 			options->control_persist_timeout = value2;
@@ -1583,11 +1676,17 @@ parse_keytypes:
 
 	case oTunnelDevice:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
+			    filename, linenum);
+			return -1;
+		}
 		value = a2tun(arg, &value2);
-		if (value == SSH_TUNID_ERR)
-			fatal("%.200s line %d: Bad tun device.", filename, linenum);
+		if (value == SSH_TUNID_ERR) {
+			error("%.200s line %d: Bad tun device.",
+			    filename, linenum);
+			return -1;
+		}
 		if (*activep) {
 			options->tun_local = value;
 			options->tun_remote = value2;
@@ -1611,9 +1710,11 @@ parse_keytypes:
 		goto parse_flag;
 
 	case oInclude:
-		if (cmdline)
-			fatal("Include directive not supported as a "
+		if (cmdline) {
+			error("Include directive not supported as a "
 			    "command-line option");
+			return -1;
+		}
 		value = 0;
 		while ((arg = strdelim(&s)) != NULL && *arg != '\0') {
 			/*
@@ -1623,9 +1724,11 @@ parse_keytypes:
 			 * as living in ~/.ssh for user configurations or
 			 * /etc/ssh for system ones.
 			 */
-			if (*arg == '~' && (flags & SSHCONF_USERCONF) == 0)
-				fatal("%.200s line %d: bad include path %s.",
+			if (*arg == '~' && (flags & SSHCONF_USERCONF) == 0) {
+				error("%.200s line %d: bad include path %s.",
 				    filename, linenum, arg);
+				return -1;
+			}
 			if (!path_absolute(arg) && *arg != '~') {
 				xasprintf(&arg2, "%s/%s",
 				    (flags & SSHCONF_USERCONF) ?
@@ -1639,9 +1742,11 @@ parse_keytypes:
 				    "files",filename, linenum, arg2);
 				free(arg2);
 				continue;
-			} else if (r != 0)
-				fatal("%.200s line %d: glob failed for %s.",
+			} else if (r != 0) {
+				error("%.200s line %d: glob failed for %s.",
 				    filename, linenum, arg2);
+				return -1;
+			}
 			free(arg2);
 			oactive = *activep;
 			for (i = 0; i < gl.gl_pathc; i++) {
@@ -1655,9 +1760,11 @@ parse_keytypes:
 				    (oactive ? 0 : SSHCONF_NEVERMATCH),
 				    activep, want_final_pass, depth + 1);
 				if (r != 1 && errno != ENOENT) {
-					fatal("Can't open user config file "
+					error("Can't open user config file "
 					    "%.100s: %.100s", gl.gl_pathv[i],
 					    strerror(errno));
+					globfree(&gl);
+					return -1;
 				}
 				/*
 				 * don't let Match in includes clobber the
@@ -1675,15 +1782,19 @@ parse_keytypes:
 
 	case oIPQoS:
 		arg = strdelim(&s);
-		if ((value = parse_ipqos(arg)) == -1)
-			fatal("%s line %d: Bad IPQoS value: %s",
+		if ((value = parse_ipqos(arg)) == -1) {
+			error("%s line %d: Bad IPQoS value: %s",
 			    filename, linenum, arg);
+			return -1;
+		}
 		arg = strdelim(&s);
 		if (arg == NULL)
 			value2 = value;
-		else if ((value2 = parse_ipqos(arg)) == -1)
-			fatal("%s line %d: Bad IPQoS value: %s",
+		else if ((value2 = parse_ipqos(arg)) == -1) {
+			error("%s line %d: Bad IPQoS value: %s",
 			    filename, linenum, arg);
+			return -1;
+		}
 		if (*activep) {
 			options->ip_qos_interactive = value;
 			options->ip_qos_bulk = value2;
@@ -1707,14 +1818,18 @@ parse_keytypes:
 		value = options->num_canonical_domains != 0;
 		while ((arg = strdelim(&s)) != NULL && *arg != '\0') {
 			if (!valid_domain(arg, 1, &errstr)) {
-				fatal("%s line %d: %s", filename, linenum,
+				error("%s line %d: %s", filename, linenum,
 				    errstr);
+				return -1;
 			}
 			if (!*activep || value)
 				continue;
-			if (options->num_canonical_domains >= MAX_CANON_DOMAINS)
-				fatal("%s line %d: too many hostname suffixes.",
+			if (options->num_canonical_domains >=
+			    MAX_CANON_DOMAINS) {
+				error("%s line %d: too many hostname suffixes.",
 				    filename, linenum);
+				return -1;
+			}
 			options->canonical_domains[
 			    options->num_canonical_domains++] = xstrdup(arg);
 		}
@@ -1730,18 +1845,22 @@ parse_keytypes:
 				lowercase(arg);
 				if ((arg2 = strchr(arg, ':')) == NULL ||
 				    arg2[1] == '\0') {
-					fatal("%s line %d: "
+					error("%s line %d: "
 					    "Invalid permitted CNAME \"%s\"",
 					    filename, linenum, arg);
+					return -1;
 				}
 				*arg2 = '\0';
 				arg2++;
 			}
 			if (!*activep || value)
 				continue;
-			if (options->num_permitted_cnames >= MAX_CANON_DOMAINS)
-				fatal("%s line %d: too many permitted CNAMEs.",
+			if (options->num_permitted_cnames >=
+			    MAX_CANON_DOMAINS) {
+				error("%s line %d: too many permitted CNAMEs.",
 				    filename, linenum);
+				return -1;
+			}
 			cname = options->permitted_cnames +
 			    options->num_permitted_cnames++;
 			cname->source_list = xstrdup(arg);
@@ -1764,12 +1883,17 @@ parse_keytypes:
 
 	case oStreamLocalBindMask:
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing StreamLocalBindMask argument.", filename, linenum);
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing StreamLocalBindMask "
+			    "argument.", filename, linenum);
+			return -1;
+		}
 		/* Parse mode in octal format */
 		value = strtol(arg, &endofnumber, 8);
-		if (arg == endofnumber || value < 0 || value > 0777)
-			fatal("%.200s line %d: Bad mask.", filename, linenum);
+		if (arg == endofnumber || value < 0 || value > 0777) {
+			error("%.200s line %d: Bad mask.", filename, linenum);
+			return -1;
+		}
 		options->fwd_opts.streamlocal_bind_mask = (mode_t)value;
 		break;
 
@@ -1784,12 +1908,16 @@ parse_keytypes:
 	case oFingerprintHash:
 		intptr = &options->fingerprint_hash;
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
-		if ((value = ssh_digest_alg_by_name(arg)) == -1)
-			fatal("%.200s line %d: Invalid hash algorithm \"%s\".",
+			return -1;
+		}
+		if ((value = ssh_digest_alg_by_name(arg)) == -1) {
+			error("%.200s line %d: Invalid hash algorithm \"%s\".",
 			    filename, linenum, arg);
+			return -1;
+		}
 		if (*activep && *intptr == -1)
 			*intptr = value;
 		break;
@@ -1815,17 +1943,24 @@ parse_keytypes:
 		value2 = 0; /* unlimited lifespan by default */
 		if (value == 3 && arg2 != NULL) {
 			/* allow "AddKeysToAgent confirm 5m" */
-			if ((value2 = convtime(arg2)) == -1 || value2 > INT_MAX)
-				fatal("%s line %d: invalid time value.",
+			if ((value2 = convtime(arg2)) == -1 ||
+			    value2 > INT_MAX) {
+				error("%s line %d: invalid time value.",
 				    filename, linenum);
+				return -1;
+			}
 		} else if (value == -1 && arg2 == NULL) {
-			if ((value2 = convtime(arg)) == -1 || value2 > INT_MAX)
-				fatal("%s line %d: unsupported option",
+			if ((value2 = convtime(arg)) == -1 ||
+			    value2 > INT_MAX) {
+				error("%s line %d: unsupported option",
 				    filename, linenum);
+				return -1;
+			}
 			value = 1; /* yes */
 		} else if (value == -1 || arg2 != NULL) {
-			fatal("%s line %d: unsupported option",
+			error("%s line %d: unsupported option",
 			    filename, linenum);
+			return -1;
 		}
 		if (*activep && options->add_keys_to_agent == -1) {
 			options->add_keys_to_agent = value;
@@ -1836,19 +1971,25 @@ parse_keytypes:
 	case oIdentityAgent:
 		charptr = &options->identity_agent;
 		arg = strdelim(&s);
-		if (!arg || *arg == '\0')
-			fatal("%.200s line %d: Missing argument.",
+		if (!arg || *arg == '\0') {
+			error("%.200s line %d: Missing argument.",
 			    filename, linenum);
+			return -1;
+		}
   parse_agent_path:
 		/* Extra validation if the string represents an env var. */
-		if ((arg2 = dollar_expand(&r, arg)) == NULL || r)
-			fatal("%.200s line %d: Invalid environment expansion "
+		if ((arg2 = dollar_expand(&r, arg)) == NULL || r) {
+			error("%.200s line %d: Invalid environment expansion "
 			    "%s.", filename, linenum, arg);
+			return -1;
+		}
 		free(arg2);
 		/* check for legacy environment format */
-		if (arg[0] == '$' && arg[1] != '{' && !valid_env_name(arg + 1)) {
-			fatal("%.200s line %d: Invalid environment name %s.",
+		if (arg[0] == '$' && arg[1] != '{' &&
+		    !valid_env_name(arg + 1)) {
+			error("%.200s line %d: Invalid environment name %s.",
 			    filename, linenum, arg);
+			return -1;
 		}
 		if (*activep && *charptr == NULL)
 			*charptr = xstrdup(arg);
@@ -1865,13 +2006,15 @@ parse_keytypes:
 		return 0;
 
 	default:
-		fatal_f("Unimplemented opcode %d", opcode);
+		error("%s line %d: Unimplemented opcode %d",
+		    filename, linenum, opcode);
 	}
 
 	/* Check that there is no garbage at end of line. */
 	if ((arg = strdelim(&s)) != NULL && *arg != '\0') {
-		fatal("%.200s line %d: garbage at end of line; \"%.200s\".",
+		error("%.200s line %d: garbage at end of line; \"%.200s\".",
 		    filename, linenum, arg);
+		return -1;
 	}
 	return 0;
 }
@@ -2005,7 +2148,9 @@ initialize_options(Options * options)
 	options->hostkeyalgorithms = NULL;
 	options->ca_sign_algorithms = NULL;
 	options->num_identity_files = 0;
+	memset(options->identity_keys, 0, sizeof(options->identity_keys));
 	options->num_certificate_files = 0;
+	memset(options->certificates, 0, sizeof(options->certificates));
 	options->hostname = NULL;
 	options->host_key_alias = NULL;
 	options->proxy_command = NULL;
@@ -2093,12 +2238,12 @@ fill_default_options_for_canonicalization(Options *options)
  * Called after processing other sources of option data, this fills those
  * options for which no value has been specified with their default values.
  */
-void
+int
 fill_default_options(Options * options)
 {
 	char *all_cipher, *all_mac, *all_kex, *all_key, *all_sig;
 	char *def_cipher, *def_mac, *def_kex, *def_key, *def_sig;
-	int r;
+	int ret = 0, r;
 
 	if (options->forward_agent == -1)
 		options->forward_agent = 0;
@@ -2122,7 +2267,7 @@ fill_default_options(Options * options)
 		clear_forwardings(options);
 
 	if (options->xauth_location == NULL)
-		options->xauth_location = _PATH_XAUTH;
+		options->xauth_location = xstrdup(_PATH_XAUTH);
 	if (options->fwd_opts.gateway_ports == -1)
 		options->fwd_opts.gateway_ports = 0;
 	if (options->fwd_opts.streamlocal_bind_mask == (mode_t)-1)
@@ -2280,8 +2425,10 @@ fill_default_options(Options * options)
 #define ASSEMBLE(what, defaults, all) \
 	do { \
 		if ((r = kex_assemble_names(&options->what, \
-		    defaults, all)) != 0) \
-			fatal_fr(r, "%s", #what); \
+		    defaults, all)) != 0) { \
+			error_fr(r, "%s", #what); \
+			goto fail; \
+		} \
 	} while (0)
 	ASSEMBLE(ciphers, def_cipher, all_cipher);
 	ASSEMBLE(macs, def_mac, all_mac);
@@ -2290,16 +2437,6 @@ fill_default_options(Options * options)
 	ASSEMBLE(pubkey_key_types, def_key, all_key);
 	ASSEMBLE(ca_sign_algorithms, def_sig, all_sig);
 #undef ASSEMBLE
-	free(all_cipher);
-	free(all_mac);
-	free(all_kex);
-	free(all_key);
-	free(all_sig);
-	free(def_cipher);
-	free(def_mac);
-	free(def_kex);
-	kex_default_pk_alg_filtered = def_key; /* save for later use */
-	free(def_sig);
 
 #define CLEAR_ON_NONE(v) \
 	do { \
@@ -2326,6 +2463,103 @@ fill_default_options(Options * options)
 	/* options->hostname will be set in the main program if appropriate */
 	/* options->host_key_alias should not be set by default */
 	/* options->preferred_authentications will be set in ssh */
+
+	/* success */
+	ret = 0;
+ fail:
+	free(all_cipher);
+	free(all_mac);
+	free(all_kex);
+	free(all_key);
+	free(all_sig);
+	free(def_cipher);
+	free(def_mac);
+	free(def_kex);
+	free(def_key);
+	free(def_sig);
+	return ret;
+}
+
+void
+free_options(Options *o)
+{
+	int i;
+
+	if (o == NULL)
+		return;
+
+#define FREE_ARRAY(type, n, a) \
+	do { \
+		type _i; \
+		for (_i = 0; _i < (n); _i++) \
+			free((a)[_i]); \
+	} while (0)
+
+	free(o->forward_agent_sock_path);
+	free(o->xauth_location);
+	FREE_ARRAY(u_int, o->num_log_verbose, o->log_verbose);
+	free(o->log_verbose);
+	free(o->ciphers);
+	free(o->macs);
+	free(o->hostkeyalgorithms);
+	free(o->kex_algorithms);
+	free(o->ca_sign_algorithms);
+	free(o->hostname);
+	free(o->host_key_alias);
+	free(o->proxy_command);
+	free(o->user);
+	FREE_ARRAY(u_int, o->num_system_hostfiles, o->system_hostfiles);
+	FREE_ARRAY(u_int, o->num_user_hostfiles, o->user_hostfiles);
+	free(o->preferred_authentications);
+	free(o->bind_address);
+	free(o->bind_interface);
+	free(o->pkcs11_provider);
+	free(o->sk_provider);
+	for (i = 0; i < o->num_identity_files; i++) {
+		free(o->identity_files[i]);
+		sshkey_free(o->identity_keys[i]);
+	}
+	for (i = 0; i < o->num_certificate_files; i++) {
+		free(o->certificate_files[i]);
+		sshkey_free(o->certificates[i]);
+	}
+	free(o->identity_agent);
+	for (i = 0; i < o->num_local_forwards; i++) {
+		free(o->local_forwards[i].listen_host);
+		free(o->local_forwards[i].listen_path);
+		free(o->local_forwards[i].connect_host);
+		free(o->local_forwards[i].connect_path);
+	}
+	free(o->local_forwards);
+	for (i = 0; i < o->num_remote_forwards; i++) {
+		free(o->remote_forwards[i].listen_host);
+		free(o->remote_forwards[i].listen_path);
+		free(o->remote_forwards[i].connect_host);
+		free(o->remote_forwards[i].connect_path);
+	}
+	free(o->remote_forwards);
+	free(o->stdio_forward_host);
+	FREE_ARRAY(int, o->num_send_env, o->send_env);
+	free(o->send_env);
+	FREE_ARRAY(int, o->num_setenv, o->setenv);
+	free(o->setenv);
+	free(o->control_path);
+	free(o->local_command);
+	free(o->remote_command);
+	FREE_ARRAY(int, o->num_canonical_domains, o->canonical_domains);
+	for (i = 0; i < o->num_permitted_cnames; i++) {
+		free(o->permitted_cnames[i].source_list);
+		free(o->permitted_cnames[i].target_list);
+	}
+	free(o->revoked_host_keys);
+	free(o->hostbased_key_types);
+	free(o->pubkey_key_types);
+	free(o->jump_user);
+	free(o->jump_host);
+	free(o->jump_extra);
+	free(o->ignored_unknown);
+	explicit_bzero(o, sizeof(*o));
+#undef FREE_ARRAY
 }
 
 struct fwdarg {
@@ -2565,12 +2799,12 @@ parse_jump(const char *s, Options *o, int active)
 
 		if (first) {
 			/* First argument and configuration is active */
-			if (parse_ssh_uri(cp, &user, &host, &port) == -1 ||
+			if (parse_ssh_uri(cp, &user, &host, &port) == -1 &&
 			    parse_user_host_port(cp, &user, &host, &port) != 0)
 				goto out;
 		} else {
 			/* Subsequent argument or inactive configuration */
-			if (parse_ssh_uri(cp, NULL, NULL, NULL) == -1 ||
+			if (parse_ssh_uri(cp, NULL, NULL, NULL) == -1 &&
 			    parse_user_host_port(cp, NULL, NULL, NULL) != 0)
 				goto out;
 		}
@@ -2604,12 +2838,27 @@ parse_jump(const char *s, Options *o, int active)
 int
 parse_ssh_uri(const char *uri, char **userp, char **hostp, int *portp)
 {
-	char *path;
-	int r;
+	char *user = NULL, *host = NULL, *path = NULL;
+	int r, port;
 
-	r = parse_uri("ssh", uri, userp, hostp, portp, &path);
+	r = parse_uri("ssh", uri, &user, &host, &port, &path);
 	if (r == 0 && path != NULL)
 		r = -1;		/* path not allowed */
+	if (r == 0) {
+		if (userp != NULL) {
+			*userp = user;
+			user = NULL;
+		}
+		if (hostp != NULL) {
+			*hostp = host;
+			host = NULL;
+		}
+		if (portp != NULL)
+			*portp = port;
+	}
+	free(user);
+	free(host);
+	free(path);
 	return r;
 }
 
diff --git a/readconf.h b/readconf.h
index be9154da..268dbf17 100644
--- a/readconf.h
+++ b/readconf.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.h,v 1.135 2020/10/16 13:26:13 djm Exp $ */
+/* $OpenBSD: readconf.h,v 1.136 2020/12/17 23:10:27 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo at cs.hut.fi>
@@ -205,8 +205,9 @@ const char *kex_default_pk_alg(void);
 char	*ssh_connection_hash(const char *thishost, const char *host,
     const char *portstr, const char *user);
 void     initialize_options(Options *);
-void     fill_default_options(Options *);
+int      fill_default_options(Options *);
 void	 fill_default_options_for_canonicalization(Options *);
+void	 free_options(Options *o);
 int	 process_config_line(Options *, struct passwd *, const char *,
     const char *, char *, const char *, int, int *, int);
 int	 read_config_file(const char *, struct passwd *, const char *,
diff --git a/ssh-keysign.c b/ssh-keysign.c
index 05824904..907162dc 100644
--- a/ssh-keysign.c
+++ b/ssh-keysign.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keysign.c,v 1.65 2020/10/18 11:32:02 djm Exp $ */
+/* $OpenBSD: ssh-keysign.c,v 1.66 2020/12/17 23:10:27 djm Exp $ */
 /*
  * Copyright (c) 2002 Markus Friedl.  All rights reserved.
  *
@@ -207,7 +207,7 @@ main(int argc, char **argv)
 	initialize_options(&options);
 	(void)read_config_file(_PATH_HOST_CONFIG_FILE, pw, "", "",
 	    &options, 0, NULL);
-	fill_default_options(&options);
+	(void)fill_default_options(&options);
 	if (options.enable_ssh_keysign != 1)
 		fatal("ssh-keysign not enabled in %s",
 		    _PATH_HOST_CONFIG_FILE);
diff --git a/ssh.c b/ssh.c
index f467ba2d..8cc606fd 100644
--- a/ssh.c
+++ b/ssh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.542 2020/11/12 22:38:57 dtucker Exp $ */
+/* $OpenBSD: ssh.c,v 1.543 2020/12/17 23:10:27 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo at cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -1237,7 +1237,8 @@ main(int ac, char **av)
 	}
 
 	/* Fill configuration defaults. */
-	fill_default_options(&options);
+	if (fill_default_options(&options) != 0)
+		cleanup_exit(255);
 
 	if (options.user == NULL)
 		options.user = xstrdup(pw->pw_name);

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.


More information about the openssh-commits mailing list