additional codeql fixes (#644)

* first pass at some codeql fixes

* address review feedback
This commit is contained in:
Tess Gauthier 2022-12-02 13:35:38 -05:00 committed by GitHub
parent 28387feffb
commit 76af8559d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 18 additions and 11 deletions

View File

@ -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)) {

View File

@ -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)

View File

@ -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);

View File

@ -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 */

View File

@ -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,
"users-groups-by-id@openssh.com") == 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;
}

View File

@ -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;

2
sshd.c
View File

@ -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) {
/*