diff --git a/src/agent.c b/src/agent.c index 7b7ac5c5d..85766fb21 100644 --- a/src/agent.c +++ b/src/agent.c @@ -61,6 +61,11 @@ #endif +#ifndef WOLFSSH_MAX_PASSPHRASE_SZ + #define WOLFSSH_MAX_PASSPHRASE_SZ 256 +#endif + + /* payloadSz is an estimate, but it shall be greater-than/equal-to * the actual value. */ static int PrepareMessage(WOLFSSH_AGENT_CTX* agent, word32 payloadSz) @@ -370,22 +375,28 @@ static int PostSuccess(WOLFSSH_AGENT_CTX* agent) static int PostLock(WOLFSSH_AGENT_CTX* agent, const byte* passphrase, word32 passphraseSz) { - char pp[32]; - word32 ppSz; + char* pp; + int ret = WS_SUCCESS; - (void)agent; WLOG(WS_LOG_AGENT, "Posting lock to agent %p", agent); - WOLFSSH_UNUSED(agent); - ppSz = sizeof(pp) - 1; - if (passphraseSz < ppSz) - ppSz = passphraseSz; + pp = (char*)WMALLOC(passphraseSz + 1, agent->heap, DYNTYPE_STRING); + if (pp == NULL) + ret = WS_MEMORY_E; - WMEMCPY(pp, passphrase, ppSz); - pp[ppSz] = 0; - WLOG(WS_LOG_AGENT, "Locking with passphrase '%s'", pp); + if (ret == WS_SUCCESS) { + WMEMCPY(pp, passphrase, passphraseSz); + pp[passphraseSz] = 0; +#ifdef SHOW_SECRETS + WLOG(WS_LOG_AGENT, "Locking with passphrase '%s'", pp); +#else + WLOG(WS_LOG_AGENT, "Locking with passphrase"); +#endif + ForceZero(pp, passphraseSz); + WFREE(pp, agent->heap, DYNTYPE_STRING); + } - return WS_SUCCESS; + return ret; } @@ -393,22 +404,28 @@ static int PostLock(WOLFSSH_AGENT_CTX* agent, static int PostUnlock(WOLFSSH_AGENT_CTX* agent, const byte* passphrase, word32 passphraseSz) { - char pp[32]; - word32 ppSz; + char* pp; + int ret = WS_SUCCESS; - (void)agent; WLOG(WS_LOG_AGENT, "Posting unlock to agent %p", agent); - WOLFSSH_UNUSED(agent); - ppSz = sizeof(pp) - 1; - if (passphraseSz < ppSz) - ppSz = passphraseSz; + pp = (char*)WMALLOC(passphraseSz + 1, agent->heap, DYNTYPE_STRING); + if (pp == NULL) + ret = WS_MEMORY_E; - WMEMCPY(pp, passphrase, ppSz); - pp[ppSz] = 0; - WLOG(WS_LOG_AGENT, "Unlocking with passphrase '%s'", pp); + if (ret == WS_SUCCESS) { + WMEMCPY(pp, passphrase, passphraseSz); + pp[passphraseSz] = 0; +#ifdef SHOW_SECRETS + WLOG(WS_LOG_AGENT, "Unlocking with passphrase '%s'", pp); +#else + WLOG(WS_LOG_AGENT, "Unlocking with passphrase"); +#endif + ForceZero(pp, passphraseSz); + WFREE(pp, agent->heap, DYNTYPE_STRING); + } - return WS_SUCCESS; + return ret; } @@ -672,7 +689,8 @@ static WOLFSSH_AGENT_ID* FindKeyId(WOLFSSH_AGENT_ID* id, if (ret == WS_SUCCESS) { while (id != NULL && WMEMCMP(digest, id->id, WC_SHA256_DIGEST_SIZE) != 0 && - WMEMCMP(keyBlob, id->keyBlob, keyBlobSz)) { + (id->keyBlobSz != keyBlobSz || + WMEMCMP(keyBlob, id->keyBlob, id->keyBlobSz) != 0)) { id = id->next; } } @@ -1252,7 +1270,8 @@ static int DoAgentLock(WOLFSSH_AGENT_CTX* agent, ato32(buf + begin, &passphraseSz); begin += LENGTH_SZ; - if (begin + passphraseSz > len) { + if (passphraseSz > WOLFSSH_MAX_PASSPHRASE_SZ || + begin + passphraseSz > len) { ret = WS_PARSE_E; } } @@ -1296,7 +1315,8 @@ static int DoAgentUnlock(WOLFSSH_AGENT_CTX* agent, ato32(buf + begin, &passphraseSz); begin += LENGTH_SZ; - if (begin + passphraseSz > len) + if (passphraseSz > WOLFSSH_MAX_PASSPHRASE_SZ || + begin + passphraseSz > len) ret = WS_PARSE_E; } diff --git a/src/certman.c b/src/certman.c index 661d5c5d9..b50aa180f 100644 --- a/src/certman.c +++ b/src/certman.c @@ -174,14 +174,16 @@ void wolfSSH_CERTMAN_free(WOLFSSH_CERTMAN* cm) int wolfSSH_CERTMAN_LoadRootCA_buffer(WOLFSSH_CERTMAN* cm, const unsigned char* rootCa, word32 rootCaSz) { - int ret; + int ret = WS_BAD_ARGUMENT; WLOG_ENTER(); - ret = wolfSSL_CertManagerLoadCABuffer(cm->cm, rootCa, rootCaSz, - WOLFSSL_FILETYPE_ASN1); - if (ret == WOLFSSL_SUCCESS) { - ret = WS_SUCCESS; + if (cm != NULL && rootCa != NULL && rootCaSz > 0) { + ret = wolfSSL_CertManagerLoadCABuffer(cm->cm, rootCa, rootCaSz, + WOLFSSL_FILETYPE_ASN1); + if (ret == WOLFSSL_SUCCESS) { + ret = WS_SUCCESS; + } } WLOG_LEAVE(ret); diff --git a/src/internal.c b/src/internal.c index 64db49dce..dc4f30439 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5055,16 +5055,24 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "DKI: Languages - Client to Server"); ret = GetUint32(&skipSz, buf, len, &begin); - if (ret == WS_SUCCESS) - begin += skipSz; + if (ret == WS_SUCCESS) { + if (skipSz <= len - begin) + begin += skipSz; + else + ret = WS_BUFFER_E; + } } /* Languages - Server to Client, skip */ if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "DKI: Languages - Server to Client"); ret = GetUint32(&skipSz, buf, len, &begin); - if (ret == WS_SUCCESS) - begin += skipSz; + if (ret == WS_SUCCESS) { + if (skipSz <= len - begin) + begin += skipSz; + else + ret = WS_BUFFER_E; + } } /* First KEX Packet Follows */ diff --git a/src/port.c b/src/port.c index 78814fd56..8c1255a64 100644 --- a/src/port.c +++ b/src/port.c @@ -839,7 +839,18 @@ char* wstrnstr(const char* s1, const char* s2, unsigned int n) * end of s1 including a null terminator. */ char* wstrncat(char* s1, const char* s2, size_t n) { - size_t freeSpace = n - strlen(s1) - 1; + size_t s1_len = 0; + size_t freeSpace; + + while (s1_len < n && s1[s1_len] != '\0') { + s1_len++; + } + + if (s1_len >= n) { + return NULL; + } + + freeSpace = n - s1_len - 1; if (freeSpace >= strlen(s2)) { #ifndef USE_WINDOWS_API diff --git a/src/ssh.c b/src/ssh.c index f73754e92..fced01523 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3835,7 +3835,7 @@ int wolfSSH_RealPath(const char* defaultPath, char* in, } /* Everything else is copied */ else { - if (curSz >= outSz - segSz) { + if (segSz > outSz || curSz >= outSz - segSz) { return WS_INVALID_PATH_E; } diff --git a/src/wolfscp.c b/src/wolfscp.c index 823e853fb..358071745 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -39,6 +39,9 @@ #include #include +#include +#include + #ifdef NO_INLINE #include @@ -1093,19 +1096,35 @@ static int GetScpFileSize(WOLFSSH* ssh, byte* buf, word32 bufSz, ret = WS_SCP_BAD_MSG_E; if (ret == WS_SUCCESS) { - /* replace space with newline for atoi */ - buf[spaceIdx] = '\n'; - ssh->scpFileSz = atoi((char *)(buf + idx)); + /* replace space with newline to terminate the size field, then parse + * with strtoull() which parses in 64-bit width, so a negative field + * such as "-1" wraps above UINT32_MAX and is rejected by the bound + * below instead of becoming a huge word32 size */ + char* endptr = NULL; + word64 fileSz; - /* restore space, increment idx to space */ + buf[spaceIdx] = '\n'; + errno = 0; + fileSz = (word64)strtoull((char*)(buf + idx), &endptr, 10); buf[spaceIdx] = ' '; - idx = spaceIdx; - /* eat trailing space */ - if (bufSz >= (word32)(idx + 1)) - idx++; + /* reject any parse error (e.g. ERANGE overflow), a non-numeric field + * (parse must consume every character up to the separator), and + * sizes too large for the word32 scpFileSz */ + if (errno != 0 || endptr != (char*)(buf + spaceIdx) || + fileSz > UINT32_MAX) { + ret = WS_SCP_BAD_MSG_E; + } + else { + ssh->scpFileSz = (word32)fileSz; - *inOutIdx = idx; + /* increment idx to space, then eat trailing space */ + idx = spaceIdx; + if (bufSz >= (word32)(idx + 1)) + idx++; + + *inOutIdx = idx; + } } return ret; @@ -1225,15 +1244,23 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz, /* read modification time */ if (ret == WS_SUCCESS) { - /* replace space with newline for atoi */ - buf[spaceIdx] = '\n'; - ssh->scpMTime = atoi((char*)(buf + idx)); + char* endptr = NULL; - /* restore space, increment idx past it */ + /* replace space with newline to terminate the field */ + buf[spaceIdx] = '\n'; + errno = 0; + ssh->scpMTime = (word64)strtoull((char*)(buf + idx), &endptr, 10); buf[spaceIdx] = ' '; - if (spaceIdx + 1 < bufSz) { + + /* reject any parse error (e.g. ERANGE overflow) and a non-numeric + * field, then step past the separating space */ + if (errno != 0 || endptr != (char*)(buf + spaceIdx)) { + ret = WS_SCP_TIMESTAMP_E; + } + else if (spaceIdx + 1 < bufSz) { idx = spaceIdx + 1; - } else { + } + else { ret = WS_SCP_TIMESTAMP_E; } } @@ -1264,15 +1291,21 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz, } if (ret == WS_SUCCESS) { - /* replace space with newline for atoi */ + char* endptr = NULL; + /* replace space with newline for strtoull */ buf[spaceIdx] = '\n'; - ssh->scpATime = atoi((char*)(buf + idx)); - + errno = 0; + ssh->scpATime = (word64)strtoull((char*)(buf + idx), &endptr, 10); /* restore space, increment idx past it */ buf[spaceIdx] = ' '; - if (spaceIdx + 1 < bufSz) { + + if (errno != 0 || endptr != (char*)(buf + spaceIdx)) { + ret = WS_SCP_TIMESTAMP_E; + } + else if (spaceIdx + 1 < bufSz) { idx = spaceIdx + 1; - } else { + } + else { ret = WS_SCP_TIMESTAMP_E; } }