Handle some codeQL warnings and errors (#645)

* Handle com codeql warnings and errors

* Handle additional codeql errors and warnings

* Add comment to changes made on upstream code

Co-authored-by: Tess Gauthier <[email protected]>

* Fix diplicated return statement

Co-authored-by: Tess Gauthier <[email protected]>

Author: vthiebaut10

auth-options.c

@@ -415,7 +415,7 @@ sshauthopt_parse(const char *opts, const char **errstrp)
 				goto alloc_fail;
 			}
 			l = (size_t)(tmp - opt);
-			cp[l] = '\0'; /* truncate at '=' */
+			cp[l] = '\0'; /* truncate at '=' */ // CodeQL [SM02311] false positive: l is calculated so that it is never out of bounds.
 			if (!valid_env_name(cp)) {
 				free(cp);
 				free(opt);
@@ -425,7 +425,7 @@ sshauthopt_parse(const char *opts, const char **errstrp)
 			/* Check for duplicates; XXX O(n*log(n)) */
 			for (i = 0; i < ret->nenv; i++) {
 				if (strncmp(ret->env[i], cp, l) == 0 &&
-				    ret->env[i][l] == '=')
+				    ret->env[i][l] == '=') // CodeQL [SM02311] false positive: l is calculated so that it is never out of bounds.
 					break;
 			}
 			free(cp);

auth2-pubkey.c

@@ -79,7 +79,7 @@ format_key(const struct sshkey *key)
 	char *ret, *fp = sshkey_fingerprint(key,
 	    options.fingerprint_hash, SSH_FP_DEFAULT);
 
-	xasprintf(&ret, "%s %s", sshkey_type(key), fp);
+	xasprintf(&ret, "%s %s", sshkey_type(key), fp); // CodeQL [SM02311] false positive: xasprintf handles case when fp is null.
 	free(fp);
 	return ret;
 }

contrib/win32/win32compat/misc.c

@@ -1602,7 +1602,7 @@ am_system()
 
 	if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &proc_token) == FALSE ||
 		GetTokenInformation(proc_token, TokenUser, NULL, 0, &info_len) == TRUE ||
-		(info = (TOKEN_USER*)malloc(info_len)) == NULL) {
+		(info = (TOKEN_USER*)malloc(info_len)) == NULL) {  	// CodeQL [SM02320]: GetTokenInformation will initialize info
 			fatal("unable to know if I am running as system");
 	}
 
@@ -1747,7 +1747,7 @@ get_sid(const char* name)
 			goto cleanup;
 		}
 
-		if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) { 
+		if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) { // CodeQL [SM02320]: GetTokenInformation will initialize info
 			errno = ENOMEM;
 			goto cleanup;
 		};

packet.c

@@ -1449,6 +1449,9 @@ ssh_packet_read_poll2_mux(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
 		return SSH_ERR_INTERNAL_ERROR;
 	*typep = SSH_MSG_NONE;
 	cp = sshbuf_ptr(state->input);
+	if (cp == NULL) { // fix CodeQL SM02311
+		return SSH_ERR_INTERNAL_ERROR;
+	}
 	if (state->packlen == 0) {
 		if (sshbuf_len(state->input) < 4 + 1)
 			return 0; /* packet is incomplete */

readpass.c

@@ -318,7 +318,7 @@ notify_start(int force_askpass, const char *fmt, ...)
 			fatal_f("stdfd_devnull failed");
 		closefrom(STDERR_FILENO + 1);
 		setenv("SSH_ASKPASS_PROMPT", "none", 1); /* hint to UI */
-		execlp(askpass, askpass, prompt, (char *)NULL);
+		execlp(askpass, askpass, prompt, (char *)NULL);  // CodeQL [SM01925] false positive: Command strings are controlled by application.
 		error_f("exec(%s): %s", askpass, strerror(errno));
 		_exit(1);
 		/* NOTREACHED */

regress/unittests/win32compat/file_tests.c

@@ -193,7 +193,7 @@ void file_simple_fileio()
 		retValue = lseek(f, offset, SEEK_SET);
 		ASSERT_INT_EQ(retValue, 0);
 		char *tmp = dup_str(small_read_buf);
-		ASSERT_PTR_NE(tmp, NULL);
+		ASSERT_PTR_NE(tmp, NULL); // CodeQL [SM02311] false positive: ASSERT_PTR_NE is checking if tmp is NULL.
 
 		retValue = read(f, small_read_buf, SMALL_RECV_BUF_SIZE);
 		small_read_buf[retValue] = '\0';

ssh-keygen.c

@@ -1943,7 +1943,7 @@ parse_hex_u64(const char *s, uint64_t *up)
 	unsigned long long ull;
 
 	errno = 0;
-	ull = strtoull(s, &ep, 16);
+	ull = strtoull(s, &ep, 16);  // CodeQL [SM02313] false positive: strtoull will initialize ep.
 	if (*s == '\0' || *ep != '\0')
 		fatal("Invalid certificate time: not a number");
 	if (errno == ERANGE && ull == ULONG_MAX)
@@ -3211,7 +3211,7 @@ do_download_sk(const char *skprovider, const char *device)
 		/* Save the key with the application string as the comment */
 		if (pass == NULL)
 			pass = private_key_passphrase();
-		if ((r = sshkey_save_private(key, path, pass,
+		if ((r = sshkey_save_private(key, path, pass,  // CodeQL [SM02311] false positive: private_key_passphrase() will never return null.
 		    key->sk_application, private_key_format,
 		    openssh_format_cipher, rounds)) != 0) {
 			error_r(r, "Saving key \"%s\" failed", path);
@@ -3932,7 +3932,7 @@ main(int argc, char **argv)
 	}
 
 	/* Save the key with the given passphrase and comment. */
-	if ((r = sshkey_save_private(private, identity_file, passphrase,
+	if ((r = sshkey_save_private(private, identity_file, passphrase,    // CodeQL [SM02311] false positive: private_key_passphrase() will never return null.
 	    comment, private_key_format, openssh_format_cipher, rounds)) != 0) {
 		error_r(r, "Saving key \"%s\" failed", identity_file);
 		freezero(passphrase, strlen(passphrase));

sshconnect.c

@@ -168,7 +168,7 @@ ssh_proxy_fdpass_connect(struct ssh *ssh, const char *host,
 		 * Execute the proxy command.
 		 * Note that we gave up any extra privileges above.
 		 */
-		execv(argv[0], argv);
+		execv(argv[0], argv); // CodeQL [SM01925] false positive: Command strings are controlled by application.
 		perror(argv[0]);
 		exit(1);
 	}

sshconnect2.c

@@ -247,7 +247,7 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 	/* Expand or fill in HostkeyAlgorithms */
 	all_key = sshkey_alg_list(0, 0, 1, ',');
 	if ((r = kex_assemble_names(&options.hostkeyalgorithms,
-	    kex_default_pk_alg(), all_key)) != 0)
+	    kex_default_pk_alg(), all_key)) != 0)   // CodeQL [SM02311] false positive: kex_assemble_names handle null all_key.
 		fatal_fr(r, "kex_assemble_namelist");
 	free(all_key);