diff --git a/addr.c b/addr.c index abf3e3d97..b1c08bc34 100644 --- a/addr.c +++ b/addr.c @@ -396,7 +396,7 @@ addr_pton_cidr(const char *p, struct xaddr *n, u_int *l) if ((mp = strchr(addrbuf, '/')) != NULL) { *mp = '\0'; mp++; - masklen = strtoul(mp, &cp, 10); + masklen = strtoul(mp, &cp, 10); // CodeQL [SM02313]: strtoul will initialize cp if (*mp < '0' || *mp > '9' || *cp != '\0' || masklen > 128) return -1; } diff --git a/auth2-passwd.c b/auth2-passwd.c index cc12cfbc1..b7c473618 100644 --- a/auth2-passwd.c +++ b/auth2-passwd.c @@ -66,7 +66,7 @@ userauth_passwd(struct ssh *ssh, const char *method) if (change) logit("password change not supported"); - else if (PRIVSEP(auth_password(ssh, password)) == 1) + else if (PRIVSEP(auth_password(ssh, password)) == 1) // CodeQL [SM01714] false positive: password is null terminated authenticated = 1; freezero(password, len); return authenticated; diff --git a/channels.c b/channels.c index 6d56dda3d..6b7d259a4 100644 --- a/channels.c +++ b/channels.c @@ -1175,8 +1175,9 @@ x11_open_helper(struct ssh *ssh, struct sshbuf *b) return 0; /* Parse the lengths of variable-length fields. */ - ucp = sshbuf_mutable_ptr(b); - if (ucp[0] == 0x42) { /* Byte order MSB first. */ + if ((ucp = sshbuf_mutable_ptr(b)) == NULL) // fix CodeQL SM02311 + return 0; + if (ucp[0] == 0x42) { /* Byte order MSB first. */ proto_len = 256 * ucp[6] + ucp[7]; data_len = 256 * ucp[8] + ucp[9]; } else if (ucp[0] == 0x6c) { /* Byte order LSB first. */ @@ -1291,6 +1292,10 @@ channel_decode_socks4(Channel *c, struct sshbuf *input, struct sshbuf *output) if (have < len) return 0; p = sshbuf_ptr(input); + if (p == NULL) { // fix CodeQL SM02311 + error("channel %d: invalid input", c->self); + return -1; + } need = 1; /* SOCKS4A uses an invalid IP address 0.0.0.x */ @@ -1324,7 +1329,7 @@ channel_decode_socks4(Channel *c, struct sshbuf *input, struct sshbuf *output) } have = sshbuf_len(input); p = sshbuf_ptr(input); - if (memchr(p, '\0', have) == NULL) { + if (p == NULL || memchr(p, '\0', have) == NULL) { // fix CodeQL SM02311 error("channel %d: decode socks4: unterminated user", c->self); return -1; } @@ -1342,7 +1347,7 @@ channel_decode_socks4(Channel *c, struct sshbuf *input, struct sshbuf *output) } else { /* SOCKS4A: two strings */ have = sshbuf_len(input); p = sshbuf_ptr(input); - if (memchr(p, '\0', have) == NULL) { + if (p == NULL || memchr(p, '\0', have) == NULL) { // fix CodeQL SM02311 error("channel %d: decode socks4a: host not nul " "terminated", c->self); return -1; @@ -1406,7 +1411,7 @@ channel_decode_socks5(Channel *c, struct sshbuf *input, struct sshbuf *output) debug2("channel %d: decode socks5", c->self); p = sshbuf_ptr(input); - if (p[0] != 0x05) + if (p == NULL || p[0] != 0x05) // fix CodeQL SM02311 return -1; have = sshbuf_len(input); if (!(c->flags & SSH_SOCKS5_AUTHDONE)) { @@ -1559,6 +1564,8 @@ channel_pre_dynamic(struct ssh *ssh, Channel *c) } /* try to guess the protocol */ p = sshbuf_ptr(c->input); + if (p == NULL) // fix CodeQL SM02311 + return; /* XXX sshbuf_peek_u8? */ switch (p[0]) { case 0x04: @@ -1621,6 +1628,8 @@ channel_before_prepare_io_rdynamic(struct ssh *ssh, Channel *c) return; /* try to guess the protocol */ p = sshbuf_ptr(c->output); + if (p == NULL) // fix CodeQL SM02311 + return; switch (p[0]) { case 0x04: /* switch input/output for reverse forwarding */ diff --git a/contrib/win32/win32compat/ansiprsr.c b/contrib/win32/win32compat/ansiprsr.c index a3bfaa5dc..e96a1b64d 100644 --- a/contrib/win32/win32compat/ansiprsr.c +++ b/contrib/win32/win32compat/ansiprsr.c @@ -670,7 +670,7 @@ ParseANSI(unsigned char * pszBuffer, unsigned char * pszBufferEnd, unsigned char } } else if (iParam[0] == 6) { char * szStatus = GetCursorPositionReport(); - if (respbuf != NULL) { + if (szStatus != NULL && respbuf != NULL) { *respbuf = szStatus; if (resplen != NULL) *resplen = strlen(szStatus); diff --git a/contrib/win32/win32compat/console.c b/contrib/win32/win32compat/console.c index 9bc0017e4..396bd7f63 100644 --- a/contrib/win32/win32compat/console.c +++ b/contrib/win32/win32compat/console.c @@ -182,7 +182,7 @@ ConEnterRawMode() if (FALSE == isConHostParserEnabled || !SetConsoleMode(GetConsoleOutputHandle(), dwAttributes)) /* Windows NT */ isAnsiParsingRequired = TRUE; - GetConsoleScreenBufferInfo(GetConsoleOutputHandle(), &csbi); + BOOL gcsbRet = GetConsoleScreenBufferInfo(GetConsoleOutputHandle(), &csbi); /* We track the view port, if conpty is not supported */ if (!is_conpty_supported()) @@ -192,6 +192,12 @@ ConEnterRawMode() * so that the clearscreen will not erase any lines. */ if (TRUE == isAnsiParsingRequired) { + if (gcsbRet == 0) + { + dwRet = GetLastError(); + error("GetConsoleScreenBufferInfo on GetConsoleOutputHandle() failed with %d", dwRet); + return; + } SavedViewRect = csbi.srWindow; debug("console doesn't support the ansi parsing"); } else { @@ -621,12 +627,17 @@ ConWriteString(char* pszString, int cbString) if ((needed = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, NULL, 0)) == 0 || (utf16 = malloc(needed * sizeof(wchar_t))) == NULL || (cnt = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, utf16, needed)) == 0) { - Result = (DWORD)printf_s(pszString); - } else { + const char* pszStringConst = pszString; + Result = (DWORD)printf_s(pszStringConst); + } + else { if (GetConsoleOutputHandle()) WriteConsoleW(GetConsoleOutputHandle(), utf16, cnt, &Result, 0); else - Result = (DWORD)wprintf_s(utf16); + { + const wchar_t* utf16Const = utf16; + Result = (DWORD)wprintf_s(utf16Const); + } } if (utf16) diff --git a/contrib/win32/win32compat/fileio.c b/contrib/win32/win32compat/fileio.c index b99773232..b7e4e6338 100644 --- a/contrib/win32/win32compat/fileio.c +++ b/contrib/win32/win32compat/fileio.c @@ -1189,7 +1189,7 @@ fileio_readlink(const char *path, char *buf, size_t bufsiz) } /* allocate the maximum possible size the reparse buffer size could be */ - reparse_buffer = (PREPARSE_DATA_BUFFER_SYMLINK)malloc(MAXIMUM_REPARSE_DATA_BUFFER_SIZE); + reparse_buffer = (PREPARSE_DATA_BUFFER_SYMLINK)malloc(MAXIMUM_REPARSE_DATA_BUFFER_SIZE); // CodeQL [SM02320]: DeviceIoControl will set reparse_buffer if (reparse_buffer == NULL) { errno = ENOMEM; goto cleanup; diff --git a/contrib/win32/win32compat/gss-sspi.c b/contrib/win32/win32compat/gss-sspi.c index 1113d0be3..ffee6a899 100644 --- a/contrib/win32/win32compat/gss-sspi.c +++ b/contrib/win32/win32compat/gss-sspi.c @@ -444,8 +444,10 @@ gss_acquire_cred(_Out_ OM_uint32 *minor_status, _In_opt_ gss_name_t desired_name /* determine expiration if requested */ if (time_rec != NULL) { FILETIME current_time; - SystemTimeToFileTime(¤t_time_system, ¤t_time); - *time_rec = (OM_uint32) (expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + if (SystemTimeToFileTime(¤t_time_system, ¤t_time) != 0) + *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + else + error("SystemTimeToFileTime failed with %d", GetLastError()); } /* set actual supported mechs if requested */ @@ -597,8 +599,10 @@ gss_init_sec_context( /* if requested, translate the expiration time to number of second */ if (time_rec != NULL) { FILETIME current_time; - SystemTimeToFileTime(¤t_time_system, ¤t_time); - *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + if (SystemTimeToFileTime(¤t_time_system, ¤t_time) != 0) + *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + else + error("SystemTimeToFileTime failed with %d", GetLastError()); } /* if requested, return the supported mechanism oid */ @@ -907,8 +911,10 @@ gss_accept_sec_context(_Out_ OM_uint32 * minor_status, _Inout_opt_ gss_ctx_id_t /* if requested, translate the expiration time to number of second */ if (time_rec != NULL) { FILETIME current_time; - SystemTimeToFileTime(¤t_time_system, ¤t_time); - *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + if (SystemTimeToFileTime(¤t_time_system, ¤t_time) != 0) + *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)¤t_time)->QuadPart) / 10000; + else + error("SystemTimeToFileTime failed with %d", GetLastError()); } /* only do checks on the finalized context (no continue needed) */ @@ -1072,6 +1078,11 @@ ssh_gssapi_krb5_userok(ssh_gssapi_client *client, char *name) * onto the next available method. */ struct passwd * user = getpwnam(name); + if (user == NULL) + { + error("sspi getpwnam failed to get user from user-provided, resolved user '%s'", name); + return 0; + } if (_stricmp(client->displayname.value, user->pw_name) != 0) { /* check failed */ debug("sspi user '%s' did not match user-provided, resolved user '%s'", diff --git a/contrib/win32/win32compat/inc/time.h b/contrib/win32/win32compat/inc/time.h index 1994ea4b8..80c469310 100644 --- a/contrib/win32/win32compat/inc/time.h +++ b/contrib/win32/win32compat/inc/time.h @@ -1,4 +1,9 @@ #include "crtheaders.h" #include TIME_H +#define localtime w32_localtime +#define ctime w32_ctime + struct tm *localtime_r(const time_t *, struct tm *); +struct tm *w32_localtime(const time_t* sourceTime); +char *w32_ctime(const time_t* sourceTime); diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 6e9580ce8..ce69bc08d 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -410,8 +410,10 @@ char* * stop reading until reach '\n' or the converted utf8 string length is n-1 */ do { - if (str_tmp) - free(str_tmp); + if (str_tmp) { + free(str_tmp); + str_tmp = NULL; + } if (fgetws(str_w, 2, stream) == NULL) goto cleanup; if ((str_tmp = utf16_to_utf8(str_w)) == NULL) { @@ -1485,6 +1487,28 @@ localtime_r(const time_t *timep, struct tm *result) return localtime_s(result, timep) == 0 ? result : NULL; } +struct tm * +w32_localtime(const time_t* sourceTime) +{ + struct tm* destTime = (struct tm*)malloc(sizeof(struct tm)); + if (destTime == NULL) + { + return NULL; + } + return localtime_s(destTime, sourceTime) == 0 ? destTime : NULL; +} + +char* +w32_ctime(const time_t* sourceTime) +{ + char *destTime = malloc(26); + if (destTime == NULL) + { + return NULL; + } + return ctime_s(destTime, 26, sourceTime) == 0 ? destTime : NULL; +} + void freezero(void *ptr, size_t sz) { @@ -1578,8 +1602,11 @@ 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 || - GetTokenInformation(proc_token, TokenUser, info, info_len, &info_len) == FALSE) + (info = (TOKEN_USER*)malloc(info_len)) == NULL) { + fatal("unable to know if I am running as system"); + } + + if (GetTokenInformation(proc_token, TokenUser, info, info_len, &info_len) == FALSE) fatal("unable to know if I am running as system"); if (IsWellKnownSid(info->User.Sid, WinLocalSystemSid)) @@ -1609,7 +1636,7 @@ lookup_sid(const wchar_t* name_utf16, PSID psid, DWORD * psid_len) wchar_t* name_utf16_modified = NULL; BOOL resolveAsAdminsSid = 0, r; - LookupAccountNameW(NULL, name_utf16, NULL, &sid_len, dom, &dom_len, &n_use); + LookupAccountNameW(NULL, name_utf16, NULL, &sid_len, dom, &dom_len, &n_use); // CodeQL [SM02313]: false positive n_use will not be uninitialized if (sid_len == 0 && _wcsicmp(name_utf16, L"administrators") == 0) { CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, NULL, &sid_len); @@ -1663,6 +1690,12 @@ lookup_sid(const wchar_t* name_utf16, PSID psid, DWORD * psid_len) debug3_f("local user name is same as machine name"); size_t name_size = wcslen(name_utf16) * 2U + 2U; name_utf16_modified = malloc(name_size * sizeof(wchar_t)); + if (name_utf16_modified == NULL) + { + errno = ENOMEM; + error_f("Failed to allocate memory"); + goto cleanup; + } name_utf16_modified[0] = L'\0'; wcscat_s(name_utf16_modified, name_size, name_utf16); wcscat_s(name_utf16_modified, name_size, L"\\"); @@ -1709,15 +1742,15 @@ get_sid(const char* name) } else { if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) == FALSE || - GetTokenInformation(token, TokenUser, NULL, 0, &info_len) == TRUE) { + GetTokenInformation(token, TokenUser, NULL, 0, &info_len) == TRUE) { // CodeQL [SM02320]: GetTokenInformation will initialize info errno = EOTHER; goto cleanup; } - if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) { + if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) { errno = ENOMEM; goto cleanup; - } + }; if (GetTokenInformation(token, TokenUser, info, info_len, &info_len) == FALSE) { errno = errno_from_Win32LastError(); diff --git a/contrib/win32/win32compat/shell-host.c b/contrib/win32/win32compat/shell-host.c index d6b5ff0f1..874f27c85 100644 --- a/contrib/win32/win32compat/shell-host.c +++ b/contrib/win32/win32compat/shell-host.c @@ -891,7 +891,7 @@ ProcessEvent(void *p) if (child_out == INVALID_HANDLE_VALUE || child_out == NULL) return ERROR_INVALID_PARAMETER; - GetWindowThreadProcessId(hwnd, &dwProcessId); + GetWindowThreadProcessId(hwnd, &dwProcessId); // CodeQL [SM02313]: false positive dwProcessId will not be uninitialized if (childProcessId != dwProcessId) return ERROR_SUCCESS; @@ -1114,7 +1114,7 @@ QueueEvent(DWORD event, HWND hwnd, LONG idObject, LONG idChild) consoleEvent* current = NULL; EnterCriticalSection(&criticalSection); - current = malloc(sizeof(consoleEvent)); + current = malloc(sizeof(consoleEvent)); // CodeQL [SM02320]: current struct fields initialized below if (current) { if (!head) { current->event = event; @@ -1128,7 +1128,8 @@ QueueEvent(DWORD event, HWND hwnd, LONG idObject, LONG idChild) head = current; tail = current; - } else { + } + else { current->event = event; current->hwnd = hwnd; current->idChild = idChild; diff --git a/contrib/win32/win32compat/signal_sigchld.c b/contrib/win32/win32compat/signal_sigchld.c index aa12e26c2..f533677c4 100644 --- a/contrib/win32/win32compat/signal_sigchld.c +++ b/contrib/win32/win32compat/signal_sigchld.c @@ -151,7 +151,7 @@ w32_kill(int pid, int sig) int waitpid(int pid, int *status, int options) { - DWORD index, ret, ret_id, exit_code, timeout = 0; + DWORD index, ret, ret_id, exit_code = 0, timeout = 0; HANDLE process = NULL; debug5("waitpid - pid:%d, options:%d", pid, options); diff --git a/contrib/win32/win32compat/ssh-agent/agent.c b/contrib/win32/win32compat/ssh-agent/agent.c index cd22288e1..9823d603a 100644 --- a/contrib/win32/win32compat/ssh-agent/agent.c +++ b/contrib/win32/win32compat/ssh-agent/agent.c @@ -198,7 +198,7 @@ agent_cleanup_connection(struct agent_connection* con) if (con->client_process_handle) CloseHandle(con->client_process_handle); - for (int i = 0; i < con->nsession_ids; i++) { + for (size_t i = 0; i < con->nsession_ids; i++) { sshkey_free(con->session_ids[i].key); sshbuf_free(con->session_ids[i].sid); } @@ -300,11 +300,13 @@ get_con_client_info(struct agent_connection* con) goto done; } - if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE || - (info = (TOKEN_USER*)malloc(info_len)) == NULL || - GetTokenInformation(client_primary_token, TokenUser, info, info_len, &info_len) == FALSE) + if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE || // CodeQL [SM02320]: GetTokenInformation will initialize info + (info = (TOKEN_USER*)malloc(info_len)) == NULL) goto done; + if (GetTokenInformation(client_primary_token, TokenUser, info, info_len, &info_len) == FALSE) + goto done; + /* check if its localsystem */ if (IsWellKnownSid(info->User.Sid, WinLocalSystemSid)) { con->client_type = SYSTEM; diff --git a/contrib/win32/win32compat/termio.c b/contrib/win32/win32compat/termio.c index 9ecd57679..1f9ac5851 100644 --- a/contrib/win32/win32compat/termio.c +++ b/contrib/win32/win32compat/termio.c @@ -203,8 +203,11 @@ WriteThread(_In_ LPVOID lpParameter) pio->write_details.buf[pio->sync_write_status.to_transfer] = '\0'; if (0 == in_raw_mode) { wchar_t* t = utf8_to_utf16(pio->write_details.buf); - WriteConsoleW(WINHANDLE(pio), t, (DWORD)wcslen(t), 0, 0); - free(t); + if (t != NULL) + { + WriteConsoleW(WINHANDLE(pio), t, (DWORD)wcslen(t), 0, 0); + free(t); + } } else { processBuffer(WINHANDLE(pio), pio->write_details.buf, pio->sync_write_status.to_transfer, &respbuf, &resplen); /* TODO - respbuf is not null in some cases, this needs to be returned back via read stream */ diff --git a/contrib/win32/win32compat/w32-doexec.c b/contrib/win32/win32compat/w32-doexec.c index 469419714..41ea51ebe 100644 --- a/contrib/win32/win32compat/w32-doexec.c +++ b/contrib/win32/win32compat/w32-doexec.c @@ -145,9 +145,13 @@ setup_session_user_vars(wchar_t* profile_path) if (ret != ERROR_SUCCESS) { error_message = get_registry_operation_error_message(ret); - error("Unable to open Registry Key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"), error_message); if (error_message) + { + error("Unable to open Registry Key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"), error_message); free(error_message); + } + else + error("Unable to open Registry Key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER")); continue; } @@ -168,9 +172,13 @@ setup_session_user_vars(wchar_t* profile_path) } else if (ret != ERROR_SUCCESS) { error_message = get_registry_operation_error_message(ret); - error("Failed to enumerate the value for registry key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"), error_message); if (error_message) + { + error("Failed to enumerate the value for registry key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"), error_message); free(error_message); + } + else + error("Failed to enumerate the value for registry key %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER")); break; } @@ -251,7 +259,7 @@ setup_session_env(struct ssh *ssh, Session* s) _snprintf(buf, ARRAYSIZE(buf), "%s@%s", s->pw->pw_name, getenv("COMPUTERNAME")); UTF8_TO_UTF16_WITH_CLEANUP(tmp, buf); /* escape $ characters as $$ to distinguish from special prompt characters */ - for (int i = 0, j = 0; i < wcslen(tmp) && j < ARRAYSIZE(wbuf) - 1; i++) { + for (size_t i = 0, j = 0; i < wcslen(tmp) && j < ARRAYSIZE(wbuf) - 1; i++) { wbuf[j] = tmp[i]; if (wbuf[j++] == L'$') wbuf[j++] = L'$'; @@ -348,7 +356,7 @@ int do_exec_windows(struct ssh *ssh, Session *s, const char *command, int pty) { JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info; HANDLE job_dup; pid_t pid = -1; - char * shell_command_option_local = NULL; + char * shell_command_option_local = NULL, *pty_cmd_cp = NULL; size_t shell_len = 0; /*account for the quotes and null*/ shell_len = strlen(s->pw->pw_shell) + 2 + 1; @@ -387,21 +395,26 @@ int do_exec_windows(struct ssh *ssh, Session *s, const char *command, int pty) { char *pty_cmd = NULL; if (command) { size_t len = strlen(shell) + 1 + strlen(shell_command_option_local) + 1 + strlen(command) + 1; - pty_cmd = calloc(1, len); - - strcpy_s(pty_cmd, len, shell); - strcat_s(pty_cmd, len, " "); - strcat_s(pty_cmd, len, shell_command_option_local); - strcat_s(pty_cmd, len, " "); - strcat_s(pty_cmd, len, command); + pty_cmd_cp = pty_cmd = calloc(1, len); + if (pty_cmd != NULL) + { + strcpy_s(pty_cmd, len, shell); + strcat_s(pty_cmd, len, " "); + strcat_s(pty_cmd, len, shell_command_option_local); + strcat_s(pty_cmd, len, " "); + strcat_s(pty_cmd, len, command); + } } else { if (shell_arguments) { size_t len = strlen(shell) + 1 + strlen(shell_arguments) + 1; pty_cmd = calloc(1, len); - strcpy_s(pty_cmd, len, shell); - strcat_s(pty_cmd, len, " "); - strcat_s(pty_cmd, len, shell_arguments); + if (pty_cmd != NULL) + { + strcpy_s(pty_cmd, len, shell); + strcat_s(pty_cmd, len, " "); + strcat_s(pty_cmd, len, shell_arguments); + } } else pty_cmd = shell; @@ -547,6 +560,8 @@ cleanup: free(shell); if (job) CloseHandle(job); + if (pty_cmd_cp) + free(pty_cmd_cp); return ret; } diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 24ef3697c..ed9309d54 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -114,13 +114,13 @@ fd_table_initialize() /* decode fd state if any */ { char *posix_fd_state; - _dupenv_s(&posix_fd_state, NULL, POSIX_FD_STATE); + /*TODO - validate parent process - to accomodate these scenarios - * A posix parent process launches a regular process that inturn launches a posix child process * In this case the posix child process may misinterpret POSIX_FD_STATE set by grand parent */ - if (NULL != posix_fd_state) { + if ((_dupenv_s(&posix_fd_state, NULL, POSIX_FD_STATE) == 0) && (NULL != posix_fd_state)) { fd_decode_state(posix_fd_state); free(posix_fd_state); _putenv_s(POSIX_FD_STATE, ""); diff --git a/contrib/win32/win32compat/w32log.c b/contrib/win32/win32compat/w32log.c index b8cf3f9f7..98ae2949f 100644 --- a/contrib/win32/win32compat/w32log.c +++ b/contrib/win32/win32compat/w32log.c @@ -124,7 +124,7 @@ openlog_file() goto cleanup; } else { - tmp_identity = malloc(wcslen(tail) * sizeof(wchar_t)); + tmp_identity = malloc(wcslen(tail) * sizeof(wchar_t)); // CodeQL [SM01952]: false positive enough space for null terminator provided if (!tmp_identity) goto cleanup; if (wcsncpy_s(tmp_identity, wcslen(tail), tail + 1, wcslen(tail) - 5) != 0) { diff --git a/contrib/win32/win32compat/win32_groupaccess.c b/contrib/win32/win32compat/win32_groupaccess.c index 6b7481dd4..a8beecdd0 100644 --- a/contrib/win32/win32compat/win32_groupaccess.c +++ b/contrib/win32/win32compat/win32_groupaccess.c @@ -95,7 +95,7 @@ get_user_groups() DWORD group_size = 0; if (GetTokenInformation(logon_token, TokenGroups, NULL, 0, &group_size) == 0 && GetLastError() != ERROR_INSUFFICIENT_BUFFER || - (group_buf = (PTOKEN_GROUPS)malloc(group_size)) == NULL) { + (group_buf = (PTOKEN_GROUPS)malloc(group_size)) == NULL) { // CodeQL [SM02320]: GetTokenInformation will initialize group_buf debug3("%s: GetTokenInformation() failed: %d", __FUNCTION__, GetLastError()); errno = EOTHER; goto cleanup; diff --git a/ed25519.c b/ed25519.c index 767ec24d6..74caeffa9 100644 --- a/ed25519.c +++ b/ed25519.c @@ -106,7 +106,7 @@ int crypto_sign_ed25519_open( const unsigned char *pk ) { - unsigned int i; + unsigned long long i; // fix CodeQL SM01735 int ret; unsigned char t2[32]; ge25519 get1, get2; diff --git a/gss-serv.c b/gss-serv.c index 8774ca3c6..6ff98e27e 100644 --- a/gss-serv.c +++ b/gss-serv.c @@ -102,7 +102,11 @@ ssh_gssapi_acquire_cred(Gssctxt *ctx) gss_OID_set oidset; if (options.gss_strict_acceptor) { - gss_create_empty_oid_set(&status, &oidset); + if (gss_create_empty_oid_set(&status, &oidset) == GSS_S_FAILURE) // fix CodeQL SM02313 + { + error("ssh_gssapi_acquire_cred: gss_create_empty_oid_set failed"); + return (-1); + } gss_add_oid_set_member(&status, ctx->oid, &oidset); if (gethostname(lname, MAXHOSTNAMELEN)) { @@ -150,15 +154,20 @@ ssh_gssapi_supported_oids(gss_OID_set *oidset) gss_OID_set supported; gss_create_empty_oid_set(&min_status, oidset); - gss_indicate_mechs(&min_status, &supported); - + if (gss_indicate_mechs(&min_status, &supported) == GSS_S_FAILURE) // fix CodeQL SM02313 + { + error("ssh_gssapi_supported_oids: gss_indicate_mechs failed to \ + determine which underlying security mechanisms are available"); + return; + } + while (supported_mechs[i]->name != NULL) { if (GSS_ERROR(gss_test_oid_set_member(&min_status, - &supported_mechs[i]->oid, supported, &present))) + &supported_mechs[i]->oid, supported, &present))) present = 0; if (present) gss_add_oid_set_member(&min_status, - &supported_mechs[i]->oid, oidset); + &supported_mechs[i]->oid, oidset); i++; } diff --git a/kex.c b/kex.c index f9bf5a223..4010d7cf5 100644 --- a/kex.c +++ b/kex.c @@ -606,6 +606,10 @@ kex_input_kexinit(int type, u_int32_t seq, struct ssh *ssh) } ssh_dispatch_set(ssh, SSH2_MSG_KEXINIT, NULL); ptr = sshpkt_ptr(ssh, &dlen); + if (ptr == NULL) { // fix CodeQL SM02313 + error_f("kex packet pointer failure"); + return SSH_ERR_INTERNAL_ERROR; + } if ((r = sshbuf_put(kex->peer, ptr, dlen)) != 0) return r; diff --git a/kexc25519.c b/kexc25519.c index f13d766d7..44cd1ff3f 100644 --- a/kexc25519.c +++ b/kexc25519.c @@ -129,6 +129,10 @@ kex_c25519_enc(struct kex *kex, const struct sshbuf *client_blob, goto out; } client_pub = sshbuf_ptr(client_blob); + if (client_pub == NULL) { // fix CodeQL SM02313 + r = SSH_ERR_ALLOC_FAIL; + goto out; + } #ifdef DEBUG_KEXECDH dump_digest("client public key 25519:", client_pub, CURVE25519_SIZE); #endif @@ -185,7 +189,7 @@ kex_c25519_dec(struct kex *kex, const struct sshbuf *server_blob, r = SSH_ERR_ALLOC_FAIL; goto out; } - if ((r = kexc25519_shared_key_ext(kex->c25519_client_key, server_pub, + if ((r = kexc25519_shared_key_ext(kex->c25519_client_key, server_pub, // CodeQL [SM02311]: false positive server_pub will not be null buf, 0)) < 0) goto out; #ifdef DEBUG_KEXECDH diff --git a/log.c b/log.c index ddfd72d50..73e6a6bdc 100644 --- a/log.c +++ b/log.c @@ -421,7 +421,7 @@ do_log(LogLevel level, int force, const char *suffix, const char *fmt, closelog_r(&sdata); #else openlog(progname, LOG_PID, log_facility); - syslog(pri, "%.500s", fmtbuf); + syslog(pri, "%.500s", fmtbuf); // CodeQL [SM01733] false positive: not a format call closelog(); #endif } diff --git a/misc.c b/misc.c index 874e9e412..b2bee4e23 100644 --- a/misc.c +++ b/misc.c @@ -1151,7 +1151,7 @@ freeargs(arglist *args) #ifdef WINDOWS void -duplicateargs(arglist *dest, arglist *source) +duplicateargs(arglist *dest, const arglist *source) { if (!source || !dest) return; @@ -1886,7 +1886,7 @@ parse_ipqos(const char *cp) return ipqos[i].value; } /* Try parsing as an integer */ - val = strtol(cp, &ep, 0); + val = strtol(cp, &ep, 0); // CodeQL [SM02313]: strtoul will initialize ep if (*cp == '\0' || *ep != '\0' || val < 0 || val > 255) return -1; return val; @@ -2272,7 +2272,7 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir, } /* If are past the homedir then we can stop */ - if (comparehome && strcmp(homedir, buf) == 0) + if (comparehome && strcmp(homedir, buf) == 0) // CodeQL [SM01714] false positive: homedir is null terminated break; /* diff --git a/misc.h b/misc.h index 5683422ed..6bf4494b1 100644 --- a/misc.h +++ b/misc.h @@ -124,7 +124,7 @@ void replacearg(arglist *, u_int, char *, ...) __attribute__((format(printf, 3, 4))); void freeargs(arglist *); #ifdef WINDOWS -void duplicateargs(arglist *, arglist *); +void duplicateargs(arglist *, const arglist *); #endif int tun_open(int, int, char **); diff --git a/monitor.c b/monitor.c index f1f7605d0..51ab6d6c3 100644 --- a/monitor.c +++ b/monitor.c @@ -1319,13 +1319,13 @@ monitor_valid_userblob(struct ssh *ssh, const u_char *data, u_int datalen) if ((r = sshbuf_skip_string(b)) != 0 || /* service */ (r = sshbuf_get_cstring(b, &cp, NULL)) != 0) fatal_fr(r, "parse method"); - if (strcmp("publickey", cp) != 0) { - if (strcmp("publickey-hostbound-v00@openssh.com", cp) == 0) + if (strcmp("publickey", cp) != 0) { // CodeQL [SM03650]: false positive cp repopulated in previous line + if (strcmp("publickey-hostbound-v00@openssh.com", cp) == 0) // CodeQL [SM03650]: false positive cp repopulated in previous line hostbound = 1; else fail++; } - free(cp); + free(cp); // CodeQL [SM03650]: false positive cp repopulated in previous line if ((r = sshbuf_get_u8(b, &type)) != 0) fatal_fr(r, "parse pktype"); if (type == 0) @@ -1390,9 +1390,9 @@ monitor_valid_hostbasedblob(const u_char *data, u_int datalen, if ((r = sshbuf_skip_string(b)) != 0 || /* service */ (r = sshbuf_get_cstring(b, &cp, NULL)) != 0) fatal_fr(r, "parse method"); - if (strcmp(cp, "hostbased") != 0) + if (strcmp(cp, "hostbased") != 0) // CodeQL [SM01977]: false positive cp has not been previously freed, CodeQL [SM03650]: false positive cp has not been previously freed fail++; - free(cp); + free(cp); // CodeQL [SM03650]: false positive cp populated again in line 1394 if ((r = sshbuf_skip_string(b)) != 0 || /* pkalg */ (r = sshbuf_skip_string(b)) != 0) /* pkblob */ fatal_fr(r, "parse pk"); diff --git a/monitor_wrap.c b/monitor_wrap.c index bbe41c106..7b98c7372 100644 --- a/monitor_wrap.c +++ b/monitor_wrap.c @@ -618,7 +618,7 @@ mm_pty_allocate(int *ptyfd, int *ttyfd, char *namebuf, size_t namebuflen) strlcpy(namebuf, p, namebuflen); /* Possible truncation */ free(p); - if ((r = sshbuf_put(loginmsg, msg, strlen(msg))) != 0) + if ((r = sshbuf_put(loginmsg, msg, strlen(msg))) != 0) // CodeQL [SM01714] false positive: sshbuf_get_cstring null terminates msg fatal_fr(r, "put loginmsg"); free(msg); diff --git a/openbsd-compat/base64.c b/openbsd-compat/base64.c index e5faba3c5..a6b16e5c8 100644 --- a/openbsd-compat/base64.c +++ b/openbsd-compat/base64.c @@ -135,7 +135,7 @@ b64_ntop(u_char const *src, size_t srclength, char *target, size_t targsize) size_t datalength = 0; u_char input[3]; u_char output[4]; - u_int i; + size_t i; // fix CodeQL SM01735 while (2 < srclength) { input[0] = *src++; diff --git a/openbsd-compat/getopt_long.c b/openbsd-compat/getopt_long.c index 1a5001f7d..4af43e0f4 100644 --- a/openbsd-compat/getopt_long.c +++ b/openbsd-compat/getopt_long.c @@ -346,7 +346,7 @@ start: return (-1); } if (*(place = nargv[optind]) != '-' || - (place[1] == '\0' && strchr(options, '-') == NULL)) { + (place[1] == '\0' && strchr(options, '-') == NULL)) { // CodeQL [SM01947]: upstream code; place re-assigned in previous line place = EMSG; /* found non-option */ if (flags & FLAG_ALLARGS) { /* diff --git a/openbsd-compat/glob.c b/openbsd-compat/glob.c index e89151789..fe33ca8b8 100644 --- a/openbsd-compat/glob.c +++ b/openbsd-compat/glob.c @@ -536,7 +536,7 @@ glob0(const Char *pattern, glob_t *pglob, struct glob_lim *limitp) /* collapse adjacent stars to one, * to avoid exponential behavior */ - if (bufnext == patbuf || bufnext[-1] != M_ALL) + if (bufnext == patbuf || bufnext[-1] != M_ALL) // CodeQL [SM01947]: false positive bufnext is more than 1 byte *bufnext++ = M_ALL; break; default: diff --git a/openbsd-compat/strtonum.c b/openbsd-compat/strtonum.c index 130d89684..e7cb0ab37 100644 --- a/openbsd-compat/strtonum.c +++ b/openbsd-compat/strtonum.c @@ -52,7 +52,7 @@ strtonum(const char *numstr, long long minval, long long maxval, if (minval > maxval) error = INVALID; else { - ll = strtoll(numstr, &ep, 10); + ll = strtoll(numstr, &ep, 10); // CodeQL [SM02313]: strtoll will initialize ep if (numstr == ep || *ep != '\0') error = INVALID; else if ((ll == LLONG_MIN && errno == ERANGE) || ll < minval) diff --git a/packet.c b/packet.c index 3f64d2d32..00b4996ab 100644 --- a/packet.c +++ b/packet.c @@ -2633,6 +2633,8 @@ ssh_packet_send_mux(struct ssh *ssh) if (len < 6) return SSH_ERR_INTERNAL_ERROR; cp = sshbuf_mutable_ptr(state->outgoing_packet); + if (cp == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; type = cp[5]; if (ssh_packet_log_type(type)) debug3_f("type %u", type); diff --git a/readconf.c b/readconf.c index 7ba207cdc..0cfc3d2ca 100644 --- a/readconf.c +++ b/readconf.c @@ -948,13 +948,13 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host, { char *str, **charptr, *endofnumber, *keyword, *arg, *arg2, *p; char **cpptr, ***cppptr, fwdarg[256]; - u_int i, *uintptr, uvalue, max_entries = 0; + u_int *uintptr, uvalue, max_entries = 0; int r, oactive, negated, opcode, *intptr, value, value2, cmdline = 0; int remotefwd, dynamicfwd; LogLevel *log_level_ptr; SyslogFacility *log_facility_ptr; long long val64; - size_t len; + size_t len, i; // fix CodeQL SM01735 struct Forward fwd; const struct multistate *multistate_ptr; struct allowed_cname *cname; @@ -2081,7 +2081,7 @@ parse_pubkey_algos: goto out; } /* Parse mode in octal format */ - value = strtol(arg, &endofnumber, 8); + value = strtol(arg, &endofnumber, 8); // CodeQL [SM02313]: strtol initializes endofnumber if (arg == endofnumber || value < 0 || value > 0777) { error("%.200s line %d: Bad mask.", filename, linenum); goto out; diff --git a/regress/unittests/test_helper/fuzz.c b/regress/unittests/test_helper/fuzz.c index 78b36654d..982e283ef 100644 --- a/regress/unittests/test_helper/fuzz.c +++ b/regress/unittests/test_helper/fuzz.c @@ -160,7 +160,7 @@ dump(u_char *p, size_t len) size_t i, j; for (i = 0; i < len; i += 16) { - fprintf(stderr, "%.4zd: ", i); + fprintf(stderr, "%.4zu: ", i); // fix CodeQL SM01735 for (j = i; j < i + 16; j++) { if (j < len) fprintf(stderr, "%02x ", p[j]); @@ -218,7 +218,7 @@ fuzz_begin(u_int strategies, const void *p, size_t l) assert(p != NULL); assert(ret != NULL); - ret->seed = malloc(l); + ret->seed = malloc(l); // CodeQL [SM02311]: tests rely on assert for NULL checks assert(ret->seed != NULL); memcpy(ret->seed, p, l); ret->slen = l; diff --git a/regress/unittests/test_helper/test_helper.c b/regress/unittests/test_helper/test_helper.c index 0f396edb2..5d330e380 100644 --- a/regress/unittests/test_helper/test_helper.c +++ b/regress/unittests/test_helper/test_helper.c @@ -159,7 +159,7 @@ main(int argc, char **argv) /* Handle systems without __progname */ if (__progname == NULL) { __progname = strrchr(argv[0], '/'); - if (__progname == NULL || __progname[1] == '\0') + if (__progname == NULL || (__progname[0] != '\0' && __progname[1] == '\0')) // fix CodeQL SM01947 __progname = argv[0]; else __progname++; @@ -420,7 +420,7 @@ tohex(const void *_s, size_t l) assert(r != NULL); for (i = j = 0; i < l; i++) { - r[j++] = hex[(s[i] >> 4) & 0xf]; + r[j++] = hex[(s[i] >> 4) & 0xf]; // CodeQL [SM02311]: tests rely on assert for NULL checks r[j++] = hex[s[i] & 0xf]; } r[j] = '\0'; diff --git a/regress/unittests/win32compat/file_tests.c b/regress/unittests/win32compat/file_tests.c index f0c4c6d3b..10ac9a63c 100644 --- a/regress/unittests/win32compat/file_tests.c +++ b/regress/unittests/win32compat/file_tests.c @@ -193,6 +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); retValue = read(f, small_read_buf, SMALL_RECV_BUF_SIZE); small_read_buf[retValue] = '\0'; @@ -419,6 +420,8 @@ file_miscellaneous_tests() ASSERT_INT_NE(retValue, -1); char *tmp = dup_str(thishost); + if (tmp == NULL) + goto out; int len = strlen(tmp); int f = dup(STDOUT_FILENO); @@ -486,7 +489,7 @@ file_miscellaneous_tests() close(f); retValue = unlink(tmp_filename); ASSERT_INT_EQ(retValue, 0); - +out: TEST_DONE(); } diff --git a/regress/unittests/win32compat/miscellaneous_tests.c b/regress/unittests/win32compat/miscellaneous_tests.c index 305ab410f..536e2a75e 100644 --- a/regress/unittests/win32compat/miscellaneous_tests.c +++ b/regress/unittests/win32compat/miscellaneous_tests.c @@ -37,8 +37,12 @@ test_path_conversion_utilities() char *s = "c:\\testdir\\test"; char *windows_style_path = dup_str(s); + if (windows_style_path == NULL) + goto out; int len = strlen(windows_style_path); char *backup = malloc(len + 1); + if (backup == NULL) + goto out; strncpy(backup, windows_style_path, len); backup[len] = '\0'; @@ -53,8 +57,9 @@ test_path_conversion_utilities() retValue = strcmp(windows_style_path, backup); ASSERT_INT_EQ(retValue, 0); - - free(windows_style_path); +out: + if (windows_style_path) + free(windows_style_path); TEST_DONE(); } @@ -80,6 +85,8 @@ test_sanitizedpath() size_t win32prgdir_len = strlen(win32prgdir_utf8); char *tmp_path = malloc(win32prgdir_len + 2); /* 1-NULL and 1-adding "/" */ + if (tmp_path == NULL) + goto out; tmp_path[0] = '/'; strcpy(tmp_path+1, win32prgdir_utf8); tmp_path[win32prgdir_len+1] = '\0'; @@ -101,7 +108,7 @@ test_sanitizedpath() retValue = wcscmp(ret, s2); ASSERT_INT_EQ(retValue, 0); free(ret); - +out: free(win32prgdir); TEST_DONE(); @@ -118,9 +125,10 @@ test_pw() struct passwd *pw1 = NULL; char *user = dup_str(pw->pw_name); - pw1 = getpwnam(user); - ASSERT_PTR_NE(pw1, NULL); - + if (user != NULL) { + pw1 = getpwnam(user); + ASSERT_PTR_NE(pw1, NULL); + } TEST_DONE(); } diff --git a/regress/unittests/win32compat/signal_tests.c b/regress/unittests/win32compat/signal_tests.c index d05dd8c97..d6fd492d2 100644 --- a/regress/unittests/win32compat/signal_tests.c +++ b/regress/unittests/win32compat/signal_tests.c @@ -44,7 +44,7 @@ VOID TEST_RESOURCES(BOOL start) static DWORD initial_count = 0; if (start) GetProcessHandleCount(GetCurrentProcess(), &initial_count); else { - DWORD final_count; + DWORD final_count = 0; GetProcessHandleCount(GetCurrentProcess(), &final_count); ASSERT_INT_EQ(initial_count, final_count); } diff --git a/regress/unittests/win32compat/tests.c b/regress/unittests/win32compat/tests.c index 5d8374ae4..018e155e6 100644 --- a/regress/unittests/win32compat/tests.c +++ b/regress/unittests/win32compat/tests.c @@ -50,6 +50,8 @@ delete_dir_recursive(char *full_dir_path) struct dirent *dp; char mode[12]; char *tmpFullPath = malloc(PATH_MAX + 1); + if (NULL == tmpFullPath) return; + strcpy(tmpFullPath, full_dir_path); int tmpStrLen = strlen(tmpFullPath); tmpFullPath[tmpStrLen++] = '\\'; diff --git a/servconf.c b/servconf.c index eb086103b..359634f36 100644 --- a/servconf.c +++ b/servconf.c @@ -2403,7 +2403,7 @@ process_server_config_line_depth(ServerOptions *options, char *line, fatal("%s line %d: %s missing argument.", filename, linenum, keyword); /* Parse mode in octal format */ - value = strtol(arg, &p, 8); + value = strtol(arg, &p, 8); // CodeQL [SM02313]: strtol will initialize p if (arg == p || value < 0 || value > 0777) fatal("%s line %d: Invalid %s.", filename, linenum, keyword); diff --git a/sftp-client.c b/sftp-client.c index ef3040ee9..b73f770ac 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -505,35 +505,35 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests, (r = sshbuf_get_string(msg, &value, &vlen)) != 0) fatal_fr(r, "parse extension"); if (strcmp(name, "posix-rename@openssh.com") == 0 && - strcmp((char *)value, "1") == 0) { + strcmp((char *)value, "1") == 0) { // CodeQL [SM01714] false positive: value is null terminated ret->exts |= SFTP_EXT_POSIX_RENAME; known = 1; } else if (strcmp(name, "statvfs@openssh.com") == 0 && - strcmp((char *)value, "2") == 0) { + strcmp((char *)value, "2") == 0) { // CodeQL [SM03650] false positive: value has not been previously freed ret->exts |= SFTP_EXT_STATVFS; known = 1; } else if (strcmp(name, "fstatvfs@openssh.com") == 0 && - strcmp((char *)value, "2") == 0) { + strcmp((char *)value, "2") == 0) { // CodeQL [SM03650] false positive: value has not been previously freed ret->exts |= SFTP_EXT_FSTATVFS; known = 1; } else if (strcmp(name, "hardlink@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_HARDLINK; known = 1; } else if (strcmp(name, "fsync@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_FSYNC; known = 1; } else if (strcmp(name, "lsetstat@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_LSETSTAT; known = 1; } else if (strcmp(name, "limits@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_LIMITS; known = 1; } else if (strcmp(name, "expand-path@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_PATH_EXPAND; known = 1; } else if (strcmp(name, "copy-data") == 0 && @@ -548,12 +548,12 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests, } if (known) { debug2("Server supports extension \"%s\" revision %s", - name, value); + name, value); // CodeQL [SM03650] false positive: value has not been previously freed } else { debug2("Unrecognised server extension \"%s\"", name); } free(name); - free(value); + free(value); // CodeQL [SM03650] false positive: value has not been previously freed } sshbuf_free(msg); @@ -782,12 +782,12 @@ do_lsreaddir(struct sftp_conn *conn, const char *path, int print_flag, if ((r = decode_attrib(msg, &a)) != 0) { error_fr(r, "couldn't decode attrib"); free(filename); - free(longname); + free(longname); // CodeQL [SM01977]: false positive longname has not been previously freed, CodeQL [SM03650]: false positive longname has not been previously freed goto out; } if (print_flag) - mprintf("%s\n", longname); + mprintf("%s\n", longname); // CodeQL[SM03650]: false positive longname has not been previously freed /* * Directory entries should never contain '/' @@ -801,12 +801,12 @@ do_lsreaddir(struct sftp_conn *conn, const char *path, int print_flag, *dir = xreallocarray(*dir, ents + 2, sizeof(**dir)); (*dir)[ents] = xcalloc(1, sizeof(***dir)); (*dir)[ents]->filename = xstrdup(filename); - (*dir)[ents]->longname = xstrdup(longname); + (*dir)[ents]->longname = xstrdup(longname); // CodeQL [SM03650]: false positive longname has not been previously freed memcpy(&(*dir)[ents]->a, &a, sizeof(a)); (*dir)[++ents] = NULL; } free(filename); - free(longname); + free(longname); // CodeQL [SM03650]: false positive longname has not been previously freed } } status = 0; diff --git a/sftp-server.c b/sftp-server.c index b93a2980b..a32927365 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -1082,13 +1082,13 @@ process_fsetstat(u_int32_t id) if (a.flags & SSH2_FILEXFER_ATTR_SIZE) { logit("set \"%s\" size %llu", - name, (unsigned long long)a.size); + name, (unsigned long long)a.size); // CodeQL [SM02311]: false positive name will not be null because of handle_to_fd check r = ftruncate(fd, a.size); if (r == -1) status = errno_to_portable(errno); } if (a.flags & SSH2_FILEXFER_ATTR_PERMISSIONS) { - logit("set \"%s\" mode %04o", name, a.perm); + logit("set \"%s\" mode %04o", name, a.perm); // CodeQL [SM02311]: false positive name will not be null because of handle_to_fd check #ifdef HAVE_FCHMOD r = fchmod(fd, a.perm & 07777); #else @@ -1103,7 +1103,7 @@ process_fsetstat(u_int32_t id) strftime(buf, sizeof(buf), "%Y%m%d-%H:%M:%S", localtime(&t)); - logit("set \"%s\" modtime %s", name, buf); + logit("set \"%s\" modtime %s", name, buf); // CodeQL [SM02311]: false positive name will not be null because of handle_to_fd check #ifdef HAVE_FUTIMES r = futimes(fd, attrib_to_tv(&a)); #else @@ -1113,7 +1113,7 @@ process_fsetstat(u_int32_t id) status = errno_to_portable(errno); } if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) { - logit("set \"%s\" owner %lu group %lu", name, + logit("set \"%s\" owner %lu group %lu", name, // CodeQL [SM02311]: false positive name will not be null because of handle_to_fd check (u_long)a.uid, (u_long)a.gid); #ifdef HAVE_FCHOWN r = fchown(fd, a.uid, a.gid); diff --git a/sftp.c b/sftp.c index ded20d372..63bb7e39c 100644 --- a/sftp.c +++ b/sftp.c @@ -344,13 +344,15 @@ local_do_shell(const char *args) if (cygwin_path_prefix_start = strstr(args, CYGWIN_PATH_PREFIX)) { int len = strlen(cygwin_path_prefix_start) + 1; char *tmp = malloc(len); - memset(tmp, 0, len); + if (tmp != NULL) // fix CodeQL SM02313 + { + memset(tmp, 0, len); - bash_to_win_path(cygwin_path_prefix_start, tmp, len); - strcpy_s(cygwin_path_prefix_start, len, tmp); /* override the original string */ + bash_to_win_path(cygwin_path_prefix_start, tmp, len); + strcpy_s(cygwin_path_prefix_start, len, tmp); /* override the original string */ - if (tmp) free(tmp); + } } } @@ -1050,6 +1052,10 @@ do_globbed_ls(struct sftp_conn *conn, const char *path, for (nentries = 0; g.gl_pathv[nentries] != NULL; nentries++) ; /* count entries */ indices = calloc(nentries, sizeof(*indices)); + if (indices == NULL) // fix CodeQL SM02313 + { + return -1; + } for (i = 0; i < nentries; i++) indices[i] = i; @@ -1531,7 +1537,7 @@ parse_args(const char **cpp, int *ignore_errors, int *disable_echo, int *aflag, if (argc - optidx < 1) goto need_num_arg; errno = 0; - ll = strtoll(argv[optidx], &cp2, base); + ll = strtoll(argv[optidx], &cp2, base); // CodeQL [SM02313]: strtoll will initialize cp2 if (cp2 == argv[optidx] || *cp2 != '\0' || ((ll == LLONG_MIN || ll == LLONG_MAX) && errno == ERANGE) || ll < 0 || ll > UINT32_MAX) { diff --git a/ssh-ed25519-sk.c b/ssh-ed25519-sk.c index 4393ca669..75a2731f5 100644 --- a/ssh-ed25519-sk.c +++ b/ssh-ed25519-sk.c @@ -133,7 +133,7 @@ ssh_ed25519_sk_verify(const struct sshkey *key, sm = sshbuf_ptr(encoded); smlen = sshbuf_len(encoded); mlen = smlen; - if ((m = malloc(smlen)) == NULL) { + if (sm == NULL || (m = malloc(smlen)) == NULL) { // fix CodeQL SM02313 r = SSH_ERR_ALLOC_FAIL; goto out; } diff --git a/ssh-keygen.c b/ssh-keygen.c index 0fe1d7bff..99ca66c99 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -1544,7 +1544,7 @@ do_change_comment(struct passwd *pw, const char *identity_comment) private_key_format != SSHKEY_PRIVATE_OPENSSH) { error("Comments are only supported for keys stored in " "the new format (-o)."); - explicit_bzero(passphrase, strlen(passphrase)); + explicit_bzero(passphrase, strlen(passphrase)); // CodeQL [SM01714] false positive: passphrase is null terminated sshkey_free(private); exit(1); } diff --git a/ssh-pkcs11-client.c b/ssh-pkcs11-client.c index fb7ab3923..4f29168d0 100644 --- a/ssh-pkcs11-client.c +++ b/ssh-pkcs11-client.c @@ -561,7 +561,7 @@ pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp, if (labelsp) (*labelsp)[i] = label; else - free(label); + free(label); // CodeQL [SM03650]: false positive label not previously freed free(blob); } } else if (type == SSH2_AGENT_FAILURE) { diff --git a/ssh-sk.c b/ssh-sk.c index fbeb39320..c635a933a 100644 --- a/ssh-sk.c +++ b/ssh-sk.c @@ -521,6 +521,8 @@ sshsk_enroll(int type, const char *provider_path, const char *device, goto out; } else { challenge = sshbuf_ptr(challenge_buf); + if (challenge == NULL) // fix CodeQL SM02313 + goto out; challenge_len = sshbuf_len(challenge_buf); debug3_f("using explicit challenge len=%zd", challenge_len); } diff --git a/ssh_api.c b/ssh_api.c index d3c661761..6ae451ca1 100644 --- a/ssh_api.c +++ b/ssh_api.c @@ -332,6 +332,9 @@ _ssh_read_banner(struct ssh *ssh, struct sshbuf *banner) int r = 0, remote_major, remote_minor, expect_nl; size_t n, j; + if (s == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; + for (j = n = 0;;) { sshbuf_reset(banner); expect_nl = 0; diff --git a/sshbuf-getput-basic.c b/sshbuf-getput-basic.c index 5c71b0e53..78db4b700 100644 --- a/sshbuf-getput-basic.c +++ b/sshbuf-getput-basic.c @@ -39,6 +39,8 @@ sshbuf_get(struct sshbuf *buf, void *v, size_t len) if ((r = sshbuf_consume(buf, len)) < 0) return r; + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; if (v != NULL && len != 0) memcpy(v, p, len); return 0; @@ -49,11 +51,13 @@ sshbuf_get_u64(struct sshbuf *buf, u_int64_t *valp) { const u_char *p = sshbuf_ptr(buf); int r; - if ((r = sshbuf_consume(buf, 8)) < 0) return r; - if (valp != NULL) + if (valp != NULL) { + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; *valp = PEEK_U64(p); + } return 0; } @@ -65,8 +69,11 @@ sshbuf_get_u32(struct sshbuf *buf, u_int32_t *valp) if ((r = sshbuf_consume(buf, 4)) < 0) return r; - if (valp != NULL) + if (valp != NULL) { + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; *valp = PEEK_U32(p); + } return 0; } @@ -78,8 +85,11 @@ sshbuf_get_u16(struct sshbuf *buf, u_int16_t *valp) if ((r = sshbuf_consume(buf, 2)) < 0) return r; - if (valp != NULL) + if (valp != NULL) { + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; *valp = PEEK_U16(p); + } return 0; } @@ -91,8 +101,11 @@ sshbuf_get_u8(struct sshbuf *buf, u_char *valp) if ((r = sshbuf_consume(buf, 1)) < 0) return r; - if (valp != NULL) + if (valp != NULL) { + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; *valp = (u_int8_t)*p; + } return 0; } @@ -251,6 +264,8 @@ sshbuf_peek_string_direct(const struct sshbuf *buf, const u_char **valp, SSHBUF_DBG(("SSH_ERR_MESSAGE_INCOMPLETE")); return SSH_ERR_MESSAGE_INCOMPLETE; } + if (p == NULL) // fix CodeQL SM02313 + return SSH_ERR_INTERNAL_ERROR; len = PEEK_U32(p); if (len > SSHBUF_SIZE_MAX - 4) { SSHBUF_DBG(("SSH_ERR_STRING_TOO_LARGE")); diff --git a/sshbuf-misc.c b/sshbuf-misc.c index 9c5c42bba..501dfb7fb 100644 --- a/sshbuf-misc.c +++ b/sshbuf-misc.c @@ -80,7 +80,7 @@ sshbuf_dtob16(struct sshbuf *buf) if (len == 0) return strdup(""); - if (SIZE_MAX / 2 <= len || (ret = malloc(len * 2 + 1)) == NULL) + if (p == NULL || SIZE_MAX / 2 <= len || (ret = malloc(len * 2 + 1)) == NULL) // fix CodeQL SM02313 return NULL; for (i = j = 0; i < len; i++) { ret[j++] = hex[(p[i] >> 4) & 0xf]; diff --git a/sshconnect2.c b/sshconnect2.c index 3607aac58..1fa4de623 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -349,7 +349,7 @@ struct cauthctxt { #ifdef GSSAPI /* gssapi */ gss_OID_set gss_supported_mechs; - u_int mech_tried; + size_t mech_tried; // fix CodeQL SM01735 #endif /* pubkey */ struct idlist keys; diff --git a/sshkey.c b/sshkey.c index 15f4c8029..ac732807a 100644 --- a/sshkey.c +++ b/sshkey.c @@ -4118,6 +4118,10 @@ private2_uudecode(struct sshbuf *blob, struct sshbuf **decodedp) /* check preamble */ cp = sshbuf_ptr(blob); + if (cp == NULL) { // fix CodeQL SM02313 + r = SSH_ERR_INTERNAL_ERROR; + goto out; + } encoded_len = sshbuf_len(blob); #ifdef SUPPORT_CRLF