address codeQL warnings (#598)

* codeql fixes

* fix type mismatches

* fix pointers in w32_time methods

* fixes for codeQL warnings

* modify checks for codeql warnings

* add comments for codeql suppressions

* additional codeql fixes and suppressions

* add codeql fixes

* add comments for codeql

* add comments for codeql

* switch from debug to error log messages

* fix another merge conflict

fix line endings in gss-sspi.c

* add null check in channels.c

* address PR feedback

* address additional review feedback

* add CodeQL comments to common code

* fix unittest-win32compat

* fix unit test

* address review feedback

* remove suppression
This commit is contained in:
Tess Gauthier 2022-11-30 11:57:01 -05:00 committed by GitHub
parent ed6ba5aa88
commit 11e2996573
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
51 changed files with 278 additions and 126 deletions

2
addr.c
View File

@ -396,7 +396,7 @@ addr_pton_cidr(const char *p, struct xaddr *n, u_int *l)
if ((mp = strchr(addrbuf, '/')) != NULL) { if ((mp = strchr(addrbuf, '/')) != NULL) {
*mp = '\0'; *mp = '\0';
mp++; 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) if (*mp < '0' || *mp > '9' || *cp != '\0' || masklen > 128)
return -1; return -1;
} }

View File

@ -66,7 +66,7 @@ userauth_passwd(struct ssh *ssh, const char *method)
if (change) if (change)
logit("password change not supported"); 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; authenticated = 1;
freezero(password, len); freezero(password, len);
return authenticated; return authenticated;

View File

@ -1175,8 +1175,9 @@ x11_open_helper(struct ssh *ssh, struct sshbuf *b)
return 0; return 0;
/* Parse the lengths of variable-length fields. */ /* Parse the lengths of variable-length fields. */
ucp = sshbuf_mutable_ptr(b); if ((ucp = sshbuf_mutable_ptr(b)) == NULL) // fix CodeQL SM02311
if (ucp[0] == 0x42) { /* Byte order MSB first. */ return 0;
if (ucp[0] == 0x42) { /* Byte order MSB first. */
proto_len = 256 * ucp[6] + ucp[7]; proto_len = 256 * ucp[6] + ucp[7];
data_len = 256 * ucp[8] + ucp[9]; data_len = 256 * ucp[8] + ucp[9];
} else if (ucp[0] == 0x6c) { /* Byte order LSB first. */ } 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) if (have < len)
return 0; return 0;
p = sshbuf_ptr(input); p = sshbuf_ptr(input);
if (p == NULL) { // fix CodeQL SM02311
error("channel %d: invalid input", c->self);
return -1;
}
need = 1; need = 1;
/* SOCKS4A uses an invalid IP address 0.0.0.x */ /* 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); have = sshbuf_len(input);
p = sshbuf_ptr(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); error("channel %d: decode socks4: unterminated user", c->self);
return -1; return -1;
} }
@ -1342,7 +1347,7 @@ channel_decode_socks4(Channel *c, struct sshbuf *input, struct sshbuf *output)
} else { /* SOCKS4A: two strings */ } else { /* SOCKS4A: two strings */
have = sshbuf_len(input); have = sshbuf_len(input);
p = sshbuf_ptr(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 " error("channel %d: decode socks4a: host not nul "
"terminated", c->self); "terminated", c->self);
return -1; return -1;
@ -1406,7 +1411,7 @@ channel_decode_socks5(Channel *c, struct sshbuf *input, struct sshbuf *output)
debug2("channel %d: decode socks5", c->self); debug2("channel %d: decode socks5", c->self);
p = sshbuf_ptr(input); p = sshbuf_ptr(input);
if (p[0] != 0x05) if (p == NULL || p[0] != 0x05) // fix CodeQL SM02311
return -1; return -1;
have = sshbuf_len(input); have = sshbuf_len(input);
if (!(c->flags & SSH_SOCKS5_AUTHDONE)) { if (!(c->flags & SSH_SOCKS5_AUTHDONE)) {
@ -1559,6 +1564,8 @@ channel_pre_dynamic(struct ssh *ssh, Channel *c)
} }
/* try to guess the protocol */ /* try to guess the protocol */
p = sshbuf_ptr(c->input); p = sshbuf_ptr(c->input);
if (p == NULL) // fix CodeQL SM02311
return;
/* XXX sshbuf_peek_u8? */ /* XXX sshbuf_peek_u8? */
switch (p[0]) { switch (p[0]) {
case 0x04: case 0x04:
@ -1621,6 +1628,8 @@ channel_before_prepare_io_rdynamic(struct ssh *ssh, Channel *c)
return; return;
/* try to guess the protocol */ /* try to guess the protocol */
p = sshbuf_ptr(c->output); p = sshbuf_ptr(c->output);
if (p == NULL) // fix CodeQL SM02311
return;
switch (p[0]) { switch (p[0]) {
case 0x04: case 0x04:
/* switch input/output for reverse forwarding */ /* switch input/output for reverse forwarding */

View File

@ -670,7 +670,7 @@ ParseANSI(unsigned char * pszBuffer, unsigned char * pszBufferEnd, unsigned char
} }
} else if (iParam[0] == 6) { } else if (iParam[0] == 6) {
char * szStatus = GetCursorPositionReport(); char * szStatus = GetCursorPositionReport();
if (respbuf != NULL) { if (szStatus != NULL && respbuf != NULL) {
*respbuf = szStatus; *respbuf = szStatus;
if (resplen != NULL) if (resplen != NULL)
*resplen = strlen(szStatus); *resplen = strlen(szStatus);

View File

@ -182,7 +182,7 @@ ConEnterRawMode()
if (FALSE == isConHostParserEnabled || !SetConsoleMode(GetConsoleOutputHandle(), dwAttributes)) /* Windows NT */ if (FALSE == isConHostParserEnabled || !SetConsoleMode(GetConsoleOutputHandle(), dwAttributes)) /* Windows NT */
isAnsiParsingRequired = TRUE; isAnsiParsingRequired = TRUE;
GetConsoleScreenBufferInfo(GetConsoleOutputHandle(), &csbi); BOOL gcsbRet = GetConsoleScreenBufferInfo(GetConsoleOutputHandle(), &csbi);
/* We track the view port, if conpty is not supported */ /* We track the view port, if conpty is not supported */
if (!is_conpty_supported()) if (!is_conpty_supported())
@ -192,6 +192,12 @@ ConEnterRawMode()
* so that the clearscreen will not erase any lines. * so that the clearscreen will not erase any lines.
*/ */
if (TRUE == isAnsiParsingRequired) { if (TRUE == isAnsiParsingRequired) {
if (gcsbRet == 0)
{
dwRet = GetLastError();
error("GetConsoleScreenBufferInfo on GetConsoleOutputHandle() failed with %d", dwRet);
return;
}
SavedViewRect = csbi.srWindow; SavedViewRect = csbi.srWindow;
debug("console doesn't support the ansi parsing"); debug("console doesn't support the ansi parsing");
} else { } else {
@ -621,12 +627,17 @@ ConWriteString(char* pszString, int cbString)
if ((needed = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, NULL, 0)) == 0 || if ((needed = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, NULL, 0)) == 0 ||
(utf16 = malloc(needed * sizeof(wchar_t))) == NULL || (utf16 = malloc(needed * sizeof(wchar_t))) == NULL ||
(cnt = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, utf16, needed)) == 0) { (cnt = MultiByteToWideChar(CP_UTF8, 0, pszString, cbString, utf16, needed)) == 0) {
Result = (DWORD)printf_s(pszString); const char* pszStringConst = pszString;
} else { Result = (DWORD)printf_s(pszStringConst);
}
else {
if (GetConsoleOutputHandle()) if (GetConsoleOutputHandle())
WriteConsoleW(GetConsoleOutputHandle(), utf16, cnt, &Result, 0); WriteConsoleW(GetConsoleOutputHandle(), utf16, cnt, &Result, 0);
else else
Result = (DWORD)wprintf_s(utf16); {
const wchar_t* utf16Const = utf16;
Result = (DWORD)wprintf_s(utf16Const);
}
} }
if (utf16) if (utf16)

View File

@ -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 */ /* 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) { if (reparse_buffer == NULL) {
errno = ENOMEM; errno = ENOMEM;
goto cleanup; goto cleanup;

View File

@ -444,8 +444,10 @@ gss_acquire_cred(_Out_ OM_uint32 *minor_status, _In_opt_ gss_name_t desired_name
/* determine expiration if requested */ /* determine expiration if requested */
if (time_rec != NULL) { if (time_rec != NULL) {
FILETIME current_time; FILETIME current_time;
SystemTimeToFileTime(&current_time_system, &current_time); if (SystemTimeToFileTime(&current_time_system, &current_time) != 0)
*time_rec = (OM_uint32) (expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000; *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000;
else
error("SystemTimeToFileTime failed with %d", GetLastError());
} }
/* set actual supported mechs if requested */ /* 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 requested, translate the expiration time to number of second */
if (time_rec != NULL) { if (time_rec != NULL) {
FILETIME current_time; FILETIME current_time;
SystemTimeToFileTime(&current_time_system, &current_time); if (SystemTimeToFileTime(&current_time_system, &current_time) != 0)
*time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000; *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000;
else
error("SystemTimeToFileTime failed with %d", GetLastError());
} }
/* if requested, return the supported mechanism oid */ /* 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 requested, translate the expiration time to number of second */
if (time_rec != NULL) { if (time_rec != NULL) {
FILETIME current_time; FILETIME current_time;
SystemTimeToFileTime(&current_time_system, &current_time); if (SystemTimeToFileTime(&current_time_system, &current_time) != 0)
*time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000; *time_rec = (OM_uint32)(expiry.QuadPart - ((PLARGE_INTEGER)&current_time)->QuadPart) / 10000;
else
error("SystemTimeToFileTime failed with %d", GetLastError());
} }
/* only do checks on the finalized context (no continue needed) */ /* 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. * onto the next available method.
*/ */
struct passwd * user = getpwnam(name); 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) { if (_stricmp(client->displayname.value, user->pw_name) != 0) {
/* check failed */ /* check failed */
debug("sspi user '%s' did not match user-provided, resolved user '%s'", debug("sspi user '%s' did not match user-provided, resolved user '%s'",

View File

@ -1,4 +1,9 @@
#include "crtheaders.h" #include "crtheaders.h"
#include TIME_H #include TIME_H
#define localtime w32_localtime
#define ctime w32_ctime
struct tm *localtime_r(const time_t *, struct tm *); struct tm *localtime_r(const time_t *, struct tm *);
struct tm *w32_localtime(const time_t* sourceTime);
char *w32_ctime(const time_t* sourceTime);

View File

@ -410,8 +410,10 @@ char*
* stop reading until reach '\n' or the converted utf8 string length is n-1 * stop reading until reach '\n' or the converted utf8 string length is n-1
*/ */
do { do {
if (str_tmp) if (str_tmp) {
free(str_tmp); free(str_tmp);
str_tmp = NULL;
}
if (fgetws(str_w, 2, stream) == NULL) if (fgetws(str_w, 2, stream) == NULL)
goto cleanup; goto cleanup;
if ((str_tmp = utf16_to_utf8(str_w)) == NULL) { 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; 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 void
freezero(void *ptr, size_t sz) freezero(void *ptr, size_t sz)
{ {
@ -1578,8 +1602,11 @@ am_system()
if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &proc_token) == FALSE || if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &proc_token) == FALSE ||
GetTokenInformation(proc_token, TokenUser, NULL, 0, &info_len) == TRUE || GetTokenInformation(proc_token, TokenUser, NULL, 0, &info_len) == TRUE ||
(info = (TOKEN_USER*)malloc(info_len)) == NULL || (info = (TOKEN_USER*)malloc(info_len)) == NULL) {
GetTokenInformation(proc_token, TokenUser, info, info_len, &info_len) == FALSE) 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"); fatal("unable to know if I am running as system");
if (IsWellKnownSid(info->User.Sid, WinLocalSystemSid)) 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; wchar_t* name_utf16_modified = NULL;
BOOL resolveAsAdminsSid = 0, r; 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) { if (sid_len == 0 && _wcsicmp(name_utf16, L"administrators") == 0) {
CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, NULL, &sid_len); 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"); debug3_f("local user name is same as machine name");
size_t name_size = wcslen(name_utf16) * 2U + 2U; size_t name_size = wcslen(name_utf16) * 2U + 2U;
name_utf16_modified = malloc(name_size * sizeof(wchar_t)); 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'; name_utf16_modified[0] = L'\0';
wcscat_s(name_utf16_modified, name_size, name_utf16); wcscat_s(name_utf16_modified, name_size, name_utf16);
wcscat_s(name_utf16_modified, name_size, L"\\"); wcscat_s(name_utf16_modified, name_size, L"\\");
@ -1709,15 +1742,15 @@ get_sid(const char* name)
} }
else { else {
if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) == FALSE || 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; errno = EOTHER;
goto cleanup; goto cleanup;
} }
if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) { if ((info = (TOKEN_USER*)malloc(info_len)) == NULL) {
errno = ENOMEM; errno = ENOMEM;
goto cleanup; goto cleanup;
} };
if (GetTokenInformation(token, TokenUser, info, info_len, &info_len) == FALSE) { if (GetTokenInformation(token, TokenUser, info, info_len, &info_len) == FALSE) {
errno = errno_from_Win32LastError(); errno = errno_from_Win32LastError();

View File

@ -891,7 +891,7 @@ ProcessEvent(void *p)
if (child_out == INVALID_HANDLE_VALUE || child_out == NULL) if (child_out == INVALID_HANDLE_VALUE || child_out == NULL)
return ERROR_INVALID_PARAMETER; return ERROR_INVALID_PARAMETER;
GetWindowThreadProcessId(hwnd, &dwProcessId); GetWindowThreadProcessId(hwnd, &dwProcessId); // CodeQL [SM02313]: false positive dwProcessId will not be uninitialized
if (childProcessId != dwProcessId) if (childProcessId != dwProcessId)
return ERROR_SUCCESS; return ERROR_SUCCESS;
@ -1114,7 +1114,7 @@ QueueEvent(DWORD event, HWND hwnd, LONG idObject, LONG idChild)
consoleEvent* current = NULL; consoleEvent* current = NULL;
EnterCriticalSection(&criticalSection); EnterCriticalSection(&criticalSection);
current = malloc(sizeof(consoleEvent)); current = malloc(sizeof(consoleEvent)); // CodeQL [SM02320]: current struct fields initialized below
if (current) { if (current) {
if (!head) { if (!head) {
current->event = event; current->event = event;
@ -1128,7 +1128,8 @@ QueueEvent(DWORD event, HWND hwnd, LONG idObject, LONG idChild)
head = current; head = current;
tail = current; tail = current;
} else { }
else {
current->event = event; current->event = event;
current->hwnd = hwnd; current->hwnd = hwnd;
current->idChild = idChild; current->idChild = idChild;

View File

@ -151,7 +151,7 @@ w32_kill(int pid, int sig)
int int
waitpid(int pid, int *status, int options) 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; HANDLE process = NULL;
debug5("waitpid - pid:%d, options:%d", pid, options); debug5("waitpid - pid:%d, options:%d", pid, options);

View File

@ -198,7 +198,7 @@ agent_cleanup_connection(struct agent_connection* con)
if (con->client_process_handle) if (con->client_process_handle)
CloseHandle(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); sshkey_free(con->session_ids[i].key);
sshbuf_free(con->session_ids[i].sid); sshbuf_free(con->session_ids[i].sid);
} }
@ -300,11 +300,13 @@ get_con_client_info(struct agent_connection* con)
goto done; goto done;
} }
if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE || if (GetTokenInformation(client_primary_token, TokenUser, NULL, 0, &info_len) == TRUE || // CodeQL [SM02320]: GetTokenInformation will initialize info
(info = (TOKEN_USER*)malloc(info_len)) == NULL || (info = (TOKEN_USER*)malloc(info_len)) == NULL)
GetTokenInformation(client_primary_token, TokenUser, info, info_len, &info_len) == FALSE)
goto done; goto done;
if (GetTokenInformation(client_primary_token, TokenUser, info, info_len, &info_len) == FALSE)
goto done;
/* check if its localsystem */ /* check if its localsystem */
if (IsWellKnownSid(info->User.Sid, WinLocalSystemSid)) { if (IsWellKnownSid(info->User.Sid, WinLocalSystemSid)) {
con->client_type = SYSTEM; con->client_type = SYSTEM;

View File

@ -203,8 +203,11 @@ WriteThread(_In_ LPVOID lpParameter)
pio->write_details.buf[pio->sync_write_status.to_transfer] = '\0'; pio->write_details.buf[pio->sync_write_status.to_transfer] = '\0';
if (0 == in_raw_mode) { if (0 == in_raw_mode) {
wchar_t* t = utf8_to_utf16(pio->write_details.buf); wchar_t* t = utf8_to_utf16(pio->write_details.buf);
WriteConsoleW(WINHANDLE(pio), t, (DWORD)wcslen(t), 0, 0); if (t != NULL)
free(t); {
WriteConsoleW(WINHANDLE(pio), t, (DWORD)wcslen(t), 0, 0);
free(t);
}
} else { } else {
processBuffer(WINHANDLE(pio), pio->write_details.buf, pio->sync_write_status.to_transfer, &respbuf, &resplen); 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 */ /* TODO - respbuf is not null in some cases, this needs to be returned back via read stream */

View File

@ -145,9 +145,13 @@ setup_session_user_vars(wchar_t* profile_path)
if (ret != ERROR_SUCCESS) { if (ret != ERROR_SUCCESS) {
error_message = get_registry_operation_error_message(ret); 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) if (error_message)
{
error("Unable to open Registry Key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"), error_message);
free(error_message); free(error_message);
}
else
error("Unable to open Registry Key %s. %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"));
continue; continue;
} }
@ -168,9 +172,13 @@ setup_session_user_vars(wchar_t* profile_path)
} }
else if (ret != ERROR_SUCCESS) { else if (ret != ERROR_SUCCESS) {
error_message = get_registry_operation_error_message(ret); 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) 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); free(error_message);
}
else
error("Failed to enumerate the value for registry key %s", (j == 0 ? "HKEY_LOCAL_MACHINE" : "HKEY_CURRENT_USER"));
break; 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")); _snprintf(buf, ARRAYSIZE(buf), "%s@%s", s->pw->pw_name, getenv("COMPUTERNAME"));
UTF8_TO_UTF16_WITH_CLEANUP(tmp, buf); UTF8_TO_UTF16_WITH_CLEANUP(tmp, buf);
/* escape $ characters as $$ to distinguish from special prompt characters */ /* 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]; wbuf[j] = tmp[i];
if (wbuf[j++] == L'$') if (wbuf[j++] == L'$')
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; JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info;
HANDLE job_dup; HANDLE job_dup;
pid_t pid = -1; 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; size_t shell_len = 0;
/*account for the quotes and null*/ /*account for the quotes and null*/
shell_len = strlen(s->pw->pw_shell) + 2 + 1; 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; char *pty_cmd = NULL;
if (command) { if (command) {
size_t len = strlen(shell) + 1 + strlen(shell_command_option_local) + 1 + strlen(command) + 1; size_t len = strlen(shell) + 1 + strlen(shell_command_option_local) + 1 + strlen(command) + 1;
pty_cmd = calloc(1, len); pty_cmd_cp = pty_cmd = calloc(1, len);
if (pty_cmd != NULL)
strcpy_s(pty_cmd, len, shell); {
strcat_s(pty_cmd, len, " "); strcpy_s(pty_cmd, len, shell);
strcat_s(pty_cmd, len, shell_command_option_local); strcat_s(pty_cmd, len, " ");
strcat_s(pty_cmd, len, " "); strcat_s(pty_cmd, len, shell_command_option_local);
strcat_s(pty_cmd, len, command); strcat_s(pty_cmd, len, " ");
strcat_s(pty_cmd, len, command);
}
} else { } else {
if (shell_arguments) { if (shell_arguments) {
size_t len = strlen(shell) + 1 + strlen(shell_arguments) + 1; size_t len = strlen(shell) + 1 + strlen(shell_arguments) + 1;
pty_cmd = calloc(1, len); pty_cmd = calloc(1, len);
strcpy_s(pty_cmd, len, shell); if (pty_cmd != NULL)
strcat_s(pty_cmd, len, " "); {
strcat_s(pty_cmd, len, shell_arguments); strcpy_s(pty_cmd, len, shell);
strcat_s(pty_cmd, len, " ");
strcat_s(pty_cmd, len, shell_arguments);
}
} }
else else
pty_cmd = shell; pty_cmd = shell;
@ -547,6 +560,8 @@ cleanup:
free(shell); free(shell);
if (job) if (job)
CloseHandle(job); CloseHandle(job);
if (pty_cmd_cp)
free(pty_cmd_cp);
return ret; return ret;
} }

View File

@ -114,13 +114,13 @@ fd_table_initialize()
/* decode fd state if any */ /* decode fd state if any */
{ {
char *posix_fd_state; char *posix_fd_state;
_dupenv_s(&posix_fd_state, NULL, POSIX_FD_STATE);
/*TODO - validate parent process - to accomodate these scenarios - /*TODO - validate parent process - to accomodate these scenarios -
* A posix parent process launches a regular process that inturn launches a posix child process * 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 * 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); fd_decode_state(posix_fd_state);
free(posix_fd_state); free(posix_fd_state);
_putenv_s(POSIX_FD_STATE, ""); _putenv_s(POSIX_FD_STATE, "");

View File

@ -124,7 +124,7 @@ openlog_file()
goto cleanup; goto cleanup;
} }
else { 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) if (!tmp_identity)
goto cleanup; goto cleanup;
if (wcsncpy_s(tmp_identity, wcslen(tail), tail + 1, wcslen(tail) - 5) != 0) { if (wcsncpy_s(tmp_identity, wcslen(tail), tail + 1, wcslen(tail) - 5) != 0) {

View File

@ -95,7 +95,7 @@ get_user_groups()
DWORD group_size = 0; DWORD group_size = 0;
if (GetTokenInformation(logon_token, TokenGroups, NULL, 0, &group_size) == 0 if (GetTokenInformation(logon_token, TokenGroups, NULL, 0, &group_size) == 0
&& GetLastError() != ERROR_INSUFFICIENT_BUFFER || && 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()); debug3("%s: GetTokenInformation() failed: %d", __FUNCTION__, GetLastError());
errno = EOTHER; errno = EOTHER;
goto cleanup; goto cleanup;

View File

@ -106,7 +106,7 @@ int crypto_sign_ed25519_open(
const unsigned char *pk const unsigned char *pk
) )
{ {
unsigned int i; unsigned long long i; // fix CodeQL SM01735
int ret; int ret;
unsigned char t2[32]; unsigned char t2[32];
ge25519 get1, get2; ge25519 get1, get2;

View File

@ -102,7 +102,11 @@ ssh_gssapi_acquire_cred(Gssctxt *ctx)
gss_OID_set oidset; gss_OID_set oidset;
if (options.gss_strict_acceptor) { 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); gss_add_oid_set_member(&status, ctx->oid, &oidset);
if (gethostname(lname, MAXHOSTNAMELEN)) { if (gethostname(lname, MAXHOSTNAMELEN)) {
@ -150,15 +154,20 @@ ssh_gssapi_supported_oids(gss_OID_set *oidset)
gss_OID_set supported; gss_OID_set supported;
gss_create_empty_oid_set(&min_status, oidset); 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) { while (supported_mechs[i]->name != NULL) {
if (GSS_ERROR(gss_test_oid_set_member(&min_status, if (GSS_ERROR(gss_test_oid_set_member(&min_status,
&supported_mechs[i]->oid, supported, &present))) &supported_mechs[i]->oid, supported, &present)))
present = 0; present = 0;
if (present) if (present)
gss_add_oid_set_member(&min_status, gss_add_oid_set_member(&min_status,
&supported_mechs[i]->oid, oidset); &supported_mechs[i]->oid, oidset);
i++; i++;
} }

4
kex.c
View File

@ -606,6 +606,10 @@ kex_input_kexinit(int type, u_int32_t seq, struct ssh *ssh)
} }
ssh_dispatch_set(ssh, SSH2_MSG_KEXINIT, NULL); ssh_dispatch_set(ssh, SSH2_MSG_KEXINIT, NULL);
ptr = sshpkt_ptr(ssh, &dlen); 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) if ((r = sshbuf_put(kex->peer, ptr, dlen)) != 0)
return r; return r;

View File

@ -129,6 +129,10 @@ kex_c25519_enc(struct kex *kex, const struct sshbuf *client_blob,
goto out; goto out;
} }
client_pub = sshbuf_ptr(client_blob); client_pub = sshbuf_ptr(client_blob);
if (client_pub == NULL) { // fix CodeQL SM02313
r = SSH_ERR_ALLOC_FAIL;
goto out;
}
#ifdef DEBUG_KEXECDH #ifdef DEBUG_KEXECDH
dump_digest("client public key 25519:", client_pub, CURVE25519_SIZE); dump_digest("client public key 25519:", client_pub, CURVE25519_SIZE);
#endif #endif
@ -185,7 +189,7 @@ kex_c25519_dec(struct kex *kex, const struct sshbuf *server_blob,
r = SSH_ERR_ALLOC_FAIL; r = SSH_ERR_ALLOC_FAIL;
goto out; 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) buf, 0)) < 0)
goto out; goto out;
#ifdef DEBUG_KEXECDH #ifdef DEBUG_KEXECDH

2
log.c
View File

@ -421,7 +421,7 @@ do_log(LogLevel level, int force, const char *suffix, const char *fmt,
closelog_r(&sdata); closelog_r(&sdata);
#else #else
openlog(progname, LOG_PID, log_facility); openlog(progname, LOG_PID, log_facility);
syslog(pri, "%.500s", fmtbuf); syslog(pri, "%.500s", fmtbuf); // CodeQL [SM01733] false positive: not a format call
closelog(); closelog();
#endif #endif
} }

6
misc.c
View File

@ -1151,7 +1151,7 @@ freeargs(arglist *args)
#ifdef WINDOWS #ifdef WINDOWS
void void
duplicateargs(arglist *dest, arglist *source) duplicateargs(arglist *dest, const arglist *source)
{ {
if (!source || !dest) if (!source || !dest)
return; return;
@ -1886,7 +1886,7 @@ parse_ipqos(const char *cp)
return ipqos[i].value; return ipqos[i].value;
} }
/* Try parsing as an integer */ /* 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) if (*cp == '\0' || *ep != '\0' || val < 0 || val > 255)
return -1; return -1;
return val; 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 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; break;
/* /*

2
misc.h
View File

@ -124,7 +124,7 @@ void replacearg(arglist *, u_int, char *, ...)
__attribute__((format(printf, 3, 4))); __attribute__((format(printf, 3, 4)));
void freeargs(arglist *); void freeargs(arglist *);
#ifdef WINDOWS #ifdef WINDOWS
void duplicateargs(arglist *, arglist *); void duplicateargs(arglist *, const arglist *);
#endif #endif
int tun_open(int, int, char **); int tun_open(int, int, char **);

View File

@ -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 */ if ((r = sshbuf_skip_string(b)) != 0 || /* service */
(r = sshbuf_get_cstring(b, &cp, NULL)) != 0) (r = sshbuf_get_cstring(b, &cp, NULL)) != 0)
fatal_fr(r, "parse method"); fatal_fr(r, "parse method");
if (strcmp("publickey", 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) if (strcmp("publickey-hostbound-v00@openssh.com", cp) == 0) // CodeQL [SM03650]: false positive cp repopulated in previous line
hostbound = 1; hostbound = 1;
else else
fail++; fail++;
} }
free(cp); free(cp); // CodeQL [SM03650]: false positive cp repopulated in previous line
if ((r = sshbuf_get_u8(b, &type)) != 0) if ((r = sshbuf_get_u8(b, &type)) != 0)
fatal_fr(r, "parse pktype"); fatal_fr(r, "parse pktype");
if (type == 0) 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 */ if ((r = sshbuf_skip_string(b)) != 0 || /* service */
(r = sshbuf_get_cstring(b, &cp, NULL)) != 0) (r = sshbuf_get_cstring(b, &cp, NULL)) != 0)
fatal_fr(r, "parse method"); 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++; fail++;
free(cp); free(cp); // CodeQL [SM03650]: false positive cp populated again in line 1394
if ((r = sshbuf_skip_string(b)) != 0 || /* pkalg */ if ((r = sshbuf_skip_string(b)) != 0 || /* pkalg */
(r = sshbuf_skip_string(b)) != 0) /* pkblob */ (r = sshbuf_skip_string(b)) != 0) /* pkblob */
fatal_fr(r, "parse pk"); fatal_fr(r, "parse pk");

View File

@ -618,7 +618,7 @@ mm_pty_allocate(int *ptyfd, int *ttyfd, char *namebuf, size_t namebuflen)
strlcpy(namebuf, p, namebuflen); /* Possible truncation */ strlcpy(namebuf, p, namebuflen); /* Possible truncation */
free(p); 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"); fatal_fr(r, "put loginmsg");
free(msg); free(msg);

View File

@ -135,7 +135,7 @@ b64_ntop(u_char const *src, size_t srclength, char *target, size_t targsize)
size_t datalength = 0; size_t datalength = 0;
u_char input[3]; u_char input[3];
u_char output[4]; u_char output[4];
u_int i; size_t i; // fix CodeQL SM01735
while (2 < srclength) { while (2 < srclength) {
input[0] = *src++; input[0] = *src++;

View File

@ -346,7 +346,7 @@ start:
return (-1); return (-1);
} }
if (*(place = nargv[optind]) != '-' || 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 */ place = EMSG; /* found non-option */
if (flags & FLAG_ALLARGS) { if (flags & FLAG_ALLARGS) {
/* /*

View File

@ -536,7 +536,7 @@ glob0(const Char *pattern, glob_t *pglob, struct glob_lim *limitp)
/* collapse adjacent stars to one, /* collapse adjacent stars to one,
* to avoid exponential behavior * 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; *bufnext++ = M_ALL;
break; break;
default: default:

View File

@ -52,7 +52,7 @@ strtonum(const char *numstr, long long minval, long long maxval,
if (minval > maxval) if (minval > maxval)
error = INVALID; error = INVALID;
else { else {
ll = strtoll(numstr, &ep, 10); ll = strtoll(numstr, &ep, 10); // CodeQL [SM02313]: strtoll will initialize ep
if (numstr == ep || *ep != '\0') if (numstr == ep || *ep != '\0')
error = INVALID; error = INVALID;
else if ((ll == LLONG_MIN && errno == ERANGE) || ll < minval) else if ((ll == LLONG_MIN && errno == ERANGE) || ll < minval)

View File

@ -2633,6 +2633,8 @@ ssh_packet_send_mux(struct ssh *ssh)
if (len < 6) if (len < 6)
return SSH_ERR_INTERNAL_ERROR; return SSH_ERR_INTERNAL_ERROR;
cp = sshbuf_mutable_ptr(state->outgoing_packet); cp = sshbuf_mutable_ptr(state->outgoing_packet);
if (cp == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
type = cp[5]; type = cp[5];
if (ssh_packet_log_type(type)) if (ssh_packet_log_type(type))
debug3_f("type %u", type); debug3_f("type %u", type);

View File

@ -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 *str, **charptr, *endofnumber, *keyword, *arg, *arg2, *p;
char **cpptr, ***cppptr, fwdarg[256]; 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 r, oactive, negated, opcode, *intptr, value, value2, cmdline = 0;
int remotefwd, dynamicfwd; int remotefwd, dynamicfwd;
LogLevel *log_level_ptr; LogLevel *log_level_ptr;
SyslogFacility *log_facility_ptr; SyslogFacility *log_facility_ptr;
long long val64; long long val64;
size_t len; size_t len, i; // fix CodeQL SM01735
struct Forward fwd; struct Forward fwd;
const struct multistate *multistate_ptr; const struct multistate *multistate_ptr;
struct allowed_cname *cname; struct allowed_cname *cname;
@ -2081,7 +2081,7 @@ parse_pubkey_algos:
goto out; goto out;
} }
/* Parse mode in octal format */ /* 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) { if (arg == endofnumber || value < 0 || value > 0777) {
error("%.200s line %d: Bad mask.", filename, linenum); error("%.200s line %d: Bad mask.", filename, linenum);
goto out; goto out;

View File

@ -160,7 +160,7 @@ dump(u_char *p, size_t len)
size_t i, j; size_t i, j;
for (i = 0; i < len; i += 16) { 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++) { for (j = i; j < i + 16; j++) {
if (j < len) if (j < len)
fprintf(stderr, "%02x ", p[j]); fprintf(stderr, "%02x ", p[j]);
@ -218,7 +218,7 @@ fuzz_begin(u_int strategies, const void *p, size_t l)
assert(p != NULL); assert(p != NULL);
assert(ret != 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); assert(ret->seed != NULL);
memcpy(ret->seed, p, l); memcpy(ret->seed, p, l);
ret->slen = l; ret->slen = l;

View File

@ -159,7 +159,7 @@ main(int argc, char **argv)
/* Handle systems without __progname */ /* Handle systems without __progname */
if (__progname == NULL) { if (__progname == NULL) {
__progname = strrchr(argv[0], '/'); __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]; __progname = argv[0];
else else
__progname++; __progname++;
@ -420,7 +420,7 @@ tohex(const void *_s, size_t l)
assert(r != NULL); assert(r != NULL);
for (i = j = 0; i < l; i++) { 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++] = hex[s[i] & 0xf];
} }
r[j] = '\0'; r[j] = '\0';

View File

@ -193,6 +193,7 @@ void file_simple_fileio()
retValue = lseek(f, offset, SEEK_SET); retValue = lseek(f, offset, SEEK_SET);
ASSERT_INT_EQ(retValue, 0); ASSERT_INT_EQ(retValue, 0);
char *tmp = dup_str(small_read_buf); char *tmp = dup_str(small_read_buf);
ASSERT_PTR_NE(tmp, NULL);
retValue = read(f, small_read_buf, SMALL_RECV_BUF_SIZE); retValue = read(f, small_read_buf, SMALL_RECV_BUF_SIZE);
small_read_buf[retValue] = '\0'; small_read_buf[retValue] = '\0';
@ -419,6 +420,8 @@ file_miscellaneous_tests()
ASSERT_INT_NE(retValue, -1); ASSERT_INT_NE(retValue, -1);
char *tmp = dup_str(thishost); char *tmp = dup_str(thishost);
if (tmp == NULL)
goto out;
int len = strlen(tmp); int len = strlen(tmp);
int f = dup(STDOUT_FILENO); int f = dup(STDOUT_FILENO);
@ -486,7 +489,7 @@ file_miscellaneous_tests()
close(f); close(f);
retValue = unlink(tmp_filename); retValue = unlink(tmp_filename);
ASSERT_INT_EQ(retValue, 0); ASSERT_INT_EQ(retValue, 0);
out:
TEST_DONE(); TEST_DONE();
} }

View File

@ -37,8 +37,12 @@ test_path_conversion_utilities()
char *s = "c:\\testdir\\test"; char *s = "c:\\testdir\\test";
char *windows_style_path = dup_str(s); char *windows_style_path = dup_str(s);
if (windows_style_path == NULL)
goto out;
int len = strlen(windows_style_path); int len = strlen(windows_style_path);
char *backup = malloc(len + 1); char *backup = malloc(len + 1);
if (backup == NULL)
goto out;
strncpy(backup, windows_style_path, len); strncpy(backup, windows_style_path, len);
backup[len] = '\0'; backup[len] = '\0';
@ -53,8 +57,9 @@ test_path_conversion_utilities()
retValue = strcmp(windows_style_path, backup); retValue = strcmp(windows_style_path, backup);
ASSERT_INT_EQ(retValue, 0); ASSERT_INT_EQ(retValue, 0);
out:
free(windows_style_path); if (windows_style_path)
free(windows_style_path);
TEST_DONE(); TEST_DONE();
} }
@ -80,6 +85,8 @@ test_sanitizedpath()
size_t win32prgdir_len = strlen(win32prgdir_utf8); size_t win32prgdir_len = strlen(win32prgdir_utf8);
char *tmp_path = malloc(win32prgdir_len + 2); /* 1-NULL and 1-adding "/" */ char *tmp_path = malloc(win32prgdir_len + 2); /* 1-NULL and 1-adding "/" */
if (tmp_path == NULL)
goto out;
tmp_path[0] = '/'; tmp_path[0] = '/';
strcpy(tmp_path+1, win32prgdir_utf8); strcpy(tmp_path+1, win32prgdir_utf8);
tmp_path[win32prgdir_len+1] = '\0'; tmp_path[win32prgdir_len+1] = '\0';
@ -101,7 +108,7 @@ test_sanitizedpath()
retValue = wcscmp(ret, s2); retValue = wcscmp(ret, s2);
ASSERT_INT_EQ(retValue, 0); ASSERT_INT_EQ(retValue, 0);
free(ret); free(ret);
out:
free(win32prgdir); free(win32prgdir);
TEST_DONE(); TEST_DONE();
@ -118,9 +125,10 @@ test_pw()
struct passwd *pw1 = NULL; struct passwd *pw1 = NULL;
char *user = dup_str(pw->pw_name); char *user = dup_str(pw->pw_name);
pw1 = getpwnam(user); if (user != NULL) {
ASSERT_PTR_NE(pw1, NULL); pw1 = getpwnam(user);
ASSERT_PTR_NE(pw1, NULL);
}
TEST_DONE(); TEST_DONE();
} }

View File

@ -44,7 +44,7 @@ VOID TEST_RESOURCES(BOOL start)
static DWORD initial_count = 0; static DWORD initial_count = 0;
if (start) GetProcessHandleCount(GetCurrentProcess(), &initial_count); if (start) GetProcessHandleCount(GetCurrentProcess(), &initial_count);
else { else {
DWORD final_count; DWORD final_count = 0;
GetProcessHandleCount(GetCurrentProcess(), &final_count); GetProcessHandleCount(GetCurrentProcess(), &final_count);
ASSERT_INT_EQ(initial_count, final_count); ASSERT_INT_EQ(initial_count, final_count);
} }

View File

@ -50,6 +50,8 @@ delete_dir_recursive(char *full_dir_path)
struct dirent *dp; struct dirent *dp;
char mode[12]; char mode[12];
char *tmpFullPath = malloc(PATH_MAX + 1); char *tmpFullPath = malloc(PATH_MAX + 1);
if (NULL == tmpFullPath) return;
strcpy(tmpFullPath, full_dir_path); strcpy(tmpFullPath, full_dir_path);
int tmpStrLen = strlen(tmpFullPath); int tmpStrLen = strlen(tmpFullPath);
tmpFullPath[tmpStrLen++] = '\\'; tmpFullPath[tmpStrLen++] = '\\';

View File

@ -2403,7 +2403,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
fatal("%s line %d: %s missing argument.", fatal("%s line %d: %s missing argument.",
filename, linenum, keyword); filename, linenum, keyword);
/* Parse mode in octal format */ /* 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) if (arg == p || value < 0 || value > 0777)
fatal("%s line %d: Invalid %s.", fatal("%s line %d: Invalid %s.",
filename, linenum, keyword); filename, linenum, keyword);

View File

@ -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) (r = sshbuf_get_string(msg, &value, &vlen)) != 0)
fatal_fr(r, "parse extension"); fatal_fr(r, "parse extension");
if (strcmp(name, "posix-rename@openssh.com") == 0 && 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; ret->exts |= SFTP_EXT_POSIX_RENAME;
known = 1; known = 1;
} else if (strcmp(name, "statvfs@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_STATVFS;
known = 1; known = 1;
} else if (strcmp(name, "fstatvfs@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_FSTATVFS;
known = 1; known = 1;
} else if (strcmp(name, "hardlink@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_HARDLINK;
known = 1; known = 1;
} else if (strcmp(name, "fsync@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_FSYNC;
known = 1; known = 1;
} else if (strcmp(name, "lsetstat@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_LSETSTAT;
known = 1; known = 1;
} else if (strcmp(name, "limits@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_LIMITS;
known = 1; known = 1;
} else if (strcmp(name, "expand-path@openssh.com") == 0 && } 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; ret->exts |= SFTP_EXT_PATH_EXPAND;
known = 1; known = 1;
} else if (strcmp(name, "copy-data") == 0 && } 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) { if (known) {
debug2("Server supports extension \"%s\" revision %s", debug2("Server supports extension \"%s\" revision %s",
name, value); name, value); // CodeQL [SM03650] false positive: value has not been previously freed
} else { } else {
debug2("Unrecognised server extension \"%s\"", name); debug2("Unrecognised server extension \"%s\"", name);
} }
free(name); free(name);
free(value); free(value); // CodeQL [SM03650] false positive: value has not been previously freed
} }
sshbuf_free(msg); 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) { if ((r = decode_attrib(msg, &a)) != 0) {
error_fr(r, "couldn't decode attrib"); error_fr(r, "couldn't decode attrib");
free(filename); 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; goto out;
} }
if (print_flag) 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 '/' * 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 = xreallocarray(*dir, ents + 2, sizeof(**dir));
(*dir)[ents] = xcalloc(1, sizeof(***dir)); (*dir)[ents] = xcalloc(1, sizeof(***dir));
(*dir)[ents]->filename = xstrdup(filename); (*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)); memcpy(&(*dir)[ents]->a, &a, sizeof(a));
(*dir)[++ents] = NULL; (*dir)[++ents] = NULL;
} }
free(filename); free(filename);
free(longname); free(longname); // CodeQL [SM03650]: false positive longname has not been previously freed
} }
} }
status = 0; status = 0;

View File

@ -1082,13 +1082,13 @@ process_fsetstat(u_int32_t id)
if (a.flags & SSH2_FILEXFER_ATTR_SIZE) { if (a.flags & SSH2_FILEXFER_ATTR_SIZE) {
logit("set \"%s\" size %llu", 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); r = ftruncate(fd, a.size);
if (r == -1) if (r == -1)
status = errno_to_portable(errno); status = errno_to_portable(errno);
} }
if (a.flags & SSH2_FILEXFER_ATTR_PERMISSIONS) { 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 #ifdef HAVE_FCHMOD
r = fchmod(fd, a.perm & 07777); r = fchmod(fd, a.perm & 07777);
#else #else
@ -1103,7 +1103,7 @@ process_fsetstat(u_int32_t id)
strftime(buf, sizeof(buf), "%Y%m%d-%H:%M:%S", strftime(buf, sizeof(buf), "%Y%m%d-%H:%M:%S",
localtime(&t)); 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 #ifdef HAVE_FUTIMES
r = futimes(fd, attrib_to_tv(&a)); r = futimes(fd, attrib_to_tv(&a));
#else #else
@ -1113,7 +1113,7 @@ process_fsetstat(u_int32_t id)
status = errno_to_portable(errno); status = errno_to_portable(errno);
} }
if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) { 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); (u_long)a.uid, (u_long)a.gid);
#ifdef HAVE_FCHOWN #ifdef HAVE_FCHOWN
r = fchown(fd, a.uid, a.gid); r = fchown(fd, a.uid, a.gid);

16
sftp.c
View File

@ -344,13 +344,15 @@ local_do_shell(const char *args)
if (cygwin_path_prefix_start = strstr(args, CYGWIN_PATH_PREFIX)) { if (cygwin_path_prefix_start = strstr(args, CYGWIN_PATH_PREFIX)) {
int len = strlen(cygwin_path_prefix_start) + 1; int len = strlen(cygwin_path_prefix_start) + 1;
char *tmp = malloc(len); 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); bash_to_win_path(cygwin_path_prefix_start, tmp, len);
strcpy_s(cygwin_path_prefix_start, len, tmp); /* override the original string */ strcpy_s(cygwin_path_prefix_start, len, tmp); /* override the original string */
if (tmp)
free(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++) for (nentries = 0; g.gl_pathv[nentries] != NULL; nentries++)
; /* count entries */ ; /* count entries */
indices = calloc(nentries, sizeof(*indices)); indices = calloc(nentries, sizeof(*indices));
if (indices == NULL) // fix CodeQL SM02313
{
return -1;
}
for (i = 0; i < nentries; i++) for (i = 0; i < nentries; i++)
indices[i] = 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) if (argc - optidx < 1)
goto need_num_arg; goto need_num_arg;
errno = 0; 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' || if (cp2 == argv[optidx] || *cp2 != '\0' ||
((ll == LLONG_MIN || ll == LLONG_MAX) && errno == ERANGE) || ((ll == LLONG_MIN || ll == LLONG_MAX) && errno == ERANGE) ||
ll < 0 || ll > UINT32_MAX) { ll < 0 || ll > UINT32_MAX) {

View File

@ -133,7 +133,7 @@ ssh_ed25519_sk_verify(const struct sshkey *key,
sm = sshbuf_ptr(encoded); sm = sshbuf_ptr(encoded);
smlen = sshbuf_len(encoded); smlen = sshbuf_len(encoded);
mlen = smlen; mlen = smlen;
if ((m = malloc(smlen)) == NULL) { if (sm == NULL || (m = malloc(smlen)) == NULL) { // fix CodeQL SM02313
r = SSH_ERR_ALLOC_FAIL; r = SSH_ERR_ALLOC_FAIL;
goto out; goto out;
} }

View File

@ -1544,7 +1544,7 @@ do_change_comment(struct passwd *pw, const char *identity_comment)
private_key_format != SSHKEY_PRIVATE_OPENSSH) { private_key_format != SSHKEY_PRIVATE_OPENSSH) {
error("Comments are only supported for keys stored in " error("Comments are only supported for keys stored in "
"the new format (-o)."); "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); sshkey_free(private);
exit(1); exit(1);
} }

View File

@ -561,7 +561,7 @@ pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp,
if (labelsp) if (labelsp)
(*labelsp)[i] = label; (*labelsp)[i] = label;
else else
free(label); free(label); // CodeQL [SM03650]: false positive label not previously freed
free(blob); free(blob);
} }
} else if (type == SSH2_AGENT_FAILURE) { } else if (type == SSH2_AGENT_FAILURE) {

View File

@ -521,6 +521,8 @@ sshsk_enroll(int type, const char *provider_path, const char *device,
goto out; goto out;
} else { } else {
challenge = sshbuf_ptr(challenge_buf); challenge = sshbuf_ptr(challenge_buf);
if (challenge == NULL) // fix CodeQL SM02313
goto out;
challenge_len = sshbuf_len(challenge_buf); challenge_len = sshbuf_len(challenge_buf);
debug3_f("using explicit challenge len=%zd", challenge_len); debug3_f("using explicit challenge len=%zd", challenge_len);
} }

View File

@ -332,6 +332,9 @@ _ssh_read_banner(struct ssh *ssh, struct sshbuf *banner)
int r = 0, remote_major, remote_minor, expect_nl; int r = 0, remote_major, remote_minor, expect_nl;
size_t n, j; size_t n, j;
if (s == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
for (j = n = 0;;) { for (j = n = 0;;) {
sshbuf_reset(banner); sshbuf_reset(banner);
expect_nl = 0; expect_nl = 0;

View File

@ -39,6 +39,8 @@ sshbuf_get(struct sshbuf *buf, void *v, size_t len)
if ((r = sshbuf_consume(buf, len)) < 0) if ((r = sshbuf_consume(buf, len)) < 0)
return r; return r;
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
if (v != NULL && len != 0) if (v != NULL && len != 0)
memcpy(v, p, len); memcpy(v, p, len);
return 0; return 0;
@ -49,11 +51,13 @@ sshbuf_get_u64(struct sshbuf *buf, u_int64_t *valp)
{ {
const u_char *p = sshbuf_ptr(buf); const u_char *p = sshbuf_ptr(buf);
int r; int r;
if ((r = sshbuf_consume(buf, 8)) < 0) if ((r = sshbuf_consume(buf, 8)) < 0)
return r; return r;
if (valp != NULL) if (valp != NULL) {
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
*valp = PEEK_U64(p); *valp = PEEK_U64(p);
}
return 0; return 0;
} }
@ -65,8 +69,11 @@ sshbuf_get_u32(struct sshbuf *buf, u_int32_t *valp)
if ((r = sshbuf_consume(buf, 4)) < 0) if ((r = sshbuf_consume(buf, 4)) < 0)
return r; return r;
if (valp != NULL) if (valp != NULL) {
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
*valp = PEEK_U32(p); *valp = PEEK_U32(p);
}
return 0; return 0;
} }
@ -78,8 +85,11 @@ sshbuf_get_u16(struct sshbuf *buf, u_int16_t *valp)
if ((r = sshbuf_consume(buf, 2)) < 0) if ((r = sshbuf_consume(buf, 2)) < 0)
return r; return r;
if (valp != NULL) if (valp != NULL) {
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
*valp = PEEK_U16(p); *valp = PEEK_U16(p);
}
return 0; return 0;
} }
@ -91,8 +101,11 @@ sshbuf_get_u8(struct sshbuf *buf, u_char *valp)
if ((r = sshbuf_consume(buf, 1)) < 0) if ((r = sshbuf_consume(buf, 1)) < 0)
return r; return r;
if (valp != NULL) if (valp != NULL) {
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
*valp = (u_int8_t)*p; *valp = (u_int8_t)*p;
}
return 0; return 0;
} }
@ -251,6 +264,8 @@ sshbuf_peek_string_direct(const struct sshbuf *buf, const u_char **valp,
SSHBUF_DBG(("SSH_ERR_MESSAGE_INCOMPLETE")); SSHBUF_DBG(("SSH_ERR_MESSAGE_INCOMPLETE"));
return SSH_ERR_MESSAGE_INCOMPLETE; return SSH_ERR_MESSAGE_INCOMPLETE;
} }
if (p == NULL) // fix CodeQL SM02313
return SSH_ERR_INTERNAL_ERROR;
len = PEEK_U32(p); len = PEEK_U32(p);
if (len > SSHBUF_SIZE_MAX - 4) { if (len > SSHBUF_SIZE_MAX - 4) {
SSHBUF_DBG(("SSH_ERR_STRING_TOO_LARGE")); SSHBUF_DBG(("SSH_ERR_STRING_TOO_LARGE"));

View File

@ -80,7 +80,7 @@ sshbuf_dtob16(struct sshbuf *buf)
if (len == 0) if (len == 0)
return strdup(""); 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; return NULL;
for (i = j = 0; i < len; i++) { for (i = j = 0; i < len; i++) {
ret[j++] = hex[(p[i] >> 4) & 0xf]; ret[j++] = hex[(p[i] >> 4) & 0xf];

View File

@ -349,7 +349,7 @@ struct cauthctxt {
#ifdef GSSAPI #ifdef GSSAPI
/* gssapi */ /* gssapi */
gss_OID_set gss_supported_mechs; gss_OID_set gss_supported_mechs;
u_int mech_tried; size_t mech_tried; // fix CodeQL SM01735
#endif #endif
/* pubkey */ /* pubkey */
struct idlist keys; struct idlist keys;

View File

@ -4118,6 +4118,10 @@ private2_uudecode(struct sshbuf *blob, struct sshbuf **decodedp)
/* check preamble */ /* check preamble */
cp = sshbuf_ptr(blob); cp = sshbuf_ptr(blob);
if (cp == NULL) { // fix CodeQL SM02313
r = SSH_ERR_INTERNAL_ERROR;
goto out;
}
encoded_len = sshbuf_len(blob); encoded_len = sshbuf_len(blob);
#ifdef SUPPORT_CRLF #ifdef SUPPORT_CRLF