additional codeql fixes (#644)

* first pass at some codeql fixes

* address review feedback

Author: tgauth

clientloop.c

@@ -2266,7 +2266,7 @@ client_input_hostkeys(struct ssh *ssh)
 		}
 		fp = sshkey_fingerprint(key, options.fingerprint_hash,
 		    SSH_FP_DEFAULT);
-		debug3_f("received %s key %s", sshkey_type(key), fp);
+		debug3_f("received %s key %s", sshkey_type(key), fp); // CodeQL [SM02311]: debug3_f can accept NULL value for fp
 		free(fp);
 
 		if (!key_accepted_by_hostkeyalgs(key)) {

contrib/win32/win32compat/ssh-agent/agent.c

@@ -300,8 +300,8 @@ get_con_client_info(struct agent_connection* con)
 		goto done;
 	}
 
-	if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE || // CodeQL [SM02320]: GetTokenInformation will initialize info
-		(info = (TOKEN_USER*)malloc(info_len)) == NULL) 
+	if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE ||
+		(info = (TOKEN_USER*)malloc(info_len)) == NULL) // CodeQL [SM02320]: GetTokenInformation will initialize info
 		goto done;
 
 	if (GetTokenInformation(client_primary_token, TokenUser, info, info_len, &info_len) == FALSE)

readconf.c

@@ -337,6 +337,7 @@ kex_default_pk_alg(void)
 		char *all_key;
 
 		all_key = sshkey_alg_list(0, 0, 1, ',');
+		if (NULL == all_key) return NULL; // fix CodeQL SM02311
 		pkalgs = match_filter_allowlist(KEX_DEFAULT_PK_ALG, all_key);
 		free(all_key);
 	}
@@ -2655,6 +2656,10 @@ fill_default_options(Options * options)
 	all_kex = kex_alg_list(',');
 	all_key = sshkey_alg_list(0, 0, 1, ',');
 	all_sig = sshkey_alg_list(0, 1, 1, ',');
+	if (NULL == all_key || NULL == all_sig) { // fix CodeQL SM02311
+		ret = SSH_ERR_INTERNAL_ERROR;
+		goto fail;
+	}
 	/* remove unsupported algos from default lists */
 	def_cipher = match_filter_allowlist(KEX_CLIENT_ENCRYPT, all_cipher);
 	def_mac = match_filter_allowlist(KEX_CLIENT_MAC, all_mac);
@@ -3281,7 +3286,7 @@ dump_client_config(Options *o, const char *host)
 	 */
 	all_key = sshkey_alg_list(0, 0, 1, ',');
 	if ((r = kex_assemble_names(&o->hostkeyalgorithms, kex_default_pk_alg(),
-	    all_key)) != 0)
+	    all_key)) != 0) // CodeQL [SM02311]: kex_assemble_names checks for NULL input
 		fatal_fr(r, "expand HostKeyAlgorithms");
 	free(all_key);
 

servconf.c

@@ -217,6 +217,7 @@ assemble_algorithms(ServerOptions *o)
 	all_kex = kex_alg_list(',');
 	all_key = sshkey_alg_list(0, 0, 1, ',');
 	all_sig = sshkey_alg_list(0, 1, 1, ',');
+	if (NULL == all_key || NULL == all_sig) goto fail; // fix CodeQL SM02311
 	/* remove unsupported algos from default lists */
 	def_cipher = match_filter_allowlist(KEX_SERVER_ENCRYPT, all_cipher);
 	def_mac = match_filter_allowlist(KEX_SERVER_MAC, all_mac);
@@ -236,6 +237,7 @@ assemble_algorithms(ServerOptions *o)
 	ASSEMBLE(pubkey_accepted_algos, def_key, all_key);
 	ASSEMBLE(ca_sign_algorithms, def_sig, all_sig);
 #undef ASSEMBLE
+fail:
 	free(all_cipher);
 	free(all_mac);
 	free(all_kex);
@@ -1048,7 +1050,7 @@ match_cfg_line(char **condition, int line, struct connection_info *ci)
 		    ci->address ? ci->address : "(null)",
 		    ci->laddress ? ci->laddress : "(null)", ci->lport);
 
-	while ((attrib = strdelim(&cp)) && *attrib != '\0') {
+	while ((attrib = strdelim(&cp)) && *attrib != '\0') { // CodeQL [SM02311]: false positive attrib is null checked
 		/* Terminate on comment */
 		if (*attrib == '#') {
 			cp = NULL; /* mark all arguments consumed */

sftp-client.c

@@ -537,12 +537,12 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests,
 			ret->exts |= SFTP_EXT_PATH_EXPAND;
 			known = 1;
 		} else if (strcmp(name, "copy-data") == 0 &&
-		    strcmp((char *)value, "1") == 0) {
+		    strcmp((char *)value, "1") == 0) { // CodeQL [SM03650] false positive: value has not been previously freed
 			ret->exts |= SFTP_EXT_COPY_DATA;
 			known = 1;
 		} else if (strcmp(name,
 		    "[email protected]") == 0 &&
-		    strcmp((char *)value, "1") == 0) {
+		    strcmp((char *)value, "1") == 0) { // CodeQL [SM03650] false positive: value has not been previously freed
 			ret->exts |= SFTP_EXT_GETUSERSGROUPS_BY_ID;
 			known = 1;
 		}

sshconnect.c

@@ -1134,7 +1134,7 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo,
 			    options.fingerprint_hash, SSH_FP_RANDOMART);
 			if (fp == NULL || ra == NULL)
 				fatal_f("sshkey_fingerprint failed");
-			logit("Host key fingerprint is %s\n%s", fp, ra);
+			logit("Host key fingerprint is %s\n%s", fp, ra); // CodeQL [SM02311]: false positive NULL check for ra in earlier line
 			free(ra);
 			free(fp);
 		}
@@ -1188,7 +1188,7 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo,
 			xextendf(&msg1, "\n", "%s key fingerprint is %s.",
 			    type, fp);
 			if (options.visual_host_key)
-				xextendf(&msg1, "\n", "%s", ra);
+				xextendf(&msg1, "\n", "%s", ra); // CodeQL [SM02311]: false positive NULL check for ra in earlier line
 			if (options.verify_host_key_dns) {
 				xextendf(&msg1, "\n",
 				    "%s host key fingerprint found in DNS.",
@@ -1640,7 +1640,7 @@ show_other_keys(struct hostkeys *hostkeys, struct sshkey *key)
 		    found->host, found->file, found->line,
 		    sshkey_type(found->key), fp);
 		if (options.visual_host_key)
-			logit("%s", ra);
+			logit("%s", ra); // CodeQL [SM02311]: false positive NULL check for ra in earlier line
 		free(ra);
 		free(fp);
 		ret = 1;

sshd.c

@@ -1190,7 +1190,7 @@ notify_hostkeys(struct ssh *ssh)
 			continue;
 		fp = sshkey_fingerprint(key, options.fingerprint_hash,
 		    SSH_FP_DEFAULT);
-		debug3_f("key %d: %s %s", i, sshkey_ssh_name(key), fp);
+		debug3_f("key %d: %s %s", i, sshkey_ssh_name(key), fp); // CodeQL [SM02311]: debug3_f can accept NULL value for fp
 		free(fp);
 		if (nkeys == 0) {
 			/*