diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index ac1adc4e0..af635bb4a 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -391,12 +391,18 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) pubKeyType[0] = 0; fp[0] = 0; - /* Get the key type out of the key. */ - ato32(pubKey, &sz); - if ((sz > pubKeySz - sizeof(word32)) - || (sz > WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE - 1)) { + /* Get the key type out of the key. Require at least a word32 + * length prefix to avoid an unsigned underflow below. */ + if (pubKeySz < (word32)sizeof(word32)) { ret = -1; } + else { + ato32(pubKey, &sz); + if ((sz > pubKeySz - sizeof(word32)) + || (sz > WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE - 1)) { + ret = -1; + } + } } if (ret == 0) { diff --git a/src/agent.c b/src/agent.c index 7b7ac5c5d..2e562001e 100644 --- a/src/agent.c +++ b/src/agent.c @@ -672,7 +672,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)) { + (keyBlobSz != id->keyBlobSz || + WMEMCMP(keyBlob, id->keyBlob, keyBlobSz) != 0)) { id = id->next; } } @@ -1386,7 +1387,7 @@ static int DoMessage(WOLFSSH_AGENT_CTX* agent, ato32(buf + begin, &payloadSz); WLOG(WS_LOG_AGENT, "payloadSz = %u", payloadSz); begin += LENGTH_SZ; - if (payloadSz > len - begin) { + if (payloadSz == 0 || payloadSz > len - begin) { ret = WS_OVERFLOW_E; } } diff --git a/src/internal.c b/src/internal.c index 8b7fd28cc..6e8ab9fb5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3509,6 +3509,12 @@ WOLFSSH_CHANNEL* ChannelNew(WOLFSSH* ssh, byte channelType, WMEMSET(newChannel, 0, sizeof(WOLFSSH_CHANNEL)); newChannel->ssh = ssh; newChannel->channelType = channelType; + /* Skip channel ids already in use to avoid collisions when + * nextChannel (word32) wraps around. */ + while (ChannelFind(ssh, ssh->nextChannel, + WS_CHANNEL_ID_SELF) != NULL) { + ssh->nextChannel++; + } newChannel->channel = ssh->nextChannel++; WLOG(WS_LOG_DEBUG, "New channel id = %u", newChannel->channel); newChannel->windowSz = initialWindowSz; @@ -4415,7 +4421,7 @@ static int GetNameList(byte* idList, word32* idListSz, */ if (ret == WS_SUCCESS) { - if (*idx >= len || *idx + 4 >= len) + if (*idx >= len || *idx + 4 > len) ret = WS_BUFFER_E; } @@ -7023,7 +7029,8 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) if (ret == WS_SUCCESS) { /* Check that scratch isn't larger than the remainder of the * sig buffer and leaves enough room for another length. */ - if (scratch > sigSz - begin - LENGTH_SZ) { + if (begin + LENGTH_SZ > sigSz || + scratch > sigSz - begin - LENGTH_SZ) { WLOG(WS_LOG_DEBUG, "sig name size is too large"); ret = WS_PARSE_E; } @@ -12362,6 +12369,10 @@ static int BuildNameList(char* buf, word32 bufSz, idx = 0; + if (srcSz == 0) { + return 0; + } + do { name = IdToName(*src); nameSz = (int)WSTRLEN(name); @@ -18773,7 +18784,12 @@ int SendChannelData(WOLFSSH* ssh, word32 channelId, word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); bound = min(bound, channel->maxPacketSz); - if (dataSz > bound) { + if (bound == 0 && dataSz != 0) { + WLOG(WS_LOG_DEBUG, "peer max packet size is zero"); + ssh->error = WS_WINDOW_FULL; + ret = WS_WINDOW_FULL; + } + else if (dataSz > bound) { WLOG(WS_LOG_DEBUG, "Trying to send %u, client will only accept %u, limiting", dataSz, bound); @@ -18886,7 +18902,12 @@ int SendChannelExtendedData(WOLFSSH* ssh, word32 channelId, word32 bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); bound = min(bound, channel->maxPacketSz); - if (dataSz > bound) { + if (bound == 0 && dataSz != 0) { + WLOG(WS_LOG_DEBUG, "peer max packet size is zero"); + ssh->error = WS_WINDOW_FULL; + ret = WS_WINDOW_FULL; + } + else if (dataSz > bound) { WLOG(WS_LOG_DEBUG, "Trying to send %u, client will only accept %u, limiting", dataSz, bound); diff --git a/src/ssh.c b/src/ssh.c index f73754e92..c3dcba3c6 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3835,7 +3835,8 @@ int wolfSSH_RealPath(const char* defaultPath, char* in, } /* Everything else is copied */ else { - if (curSz >= outSz - segSz) { + if (segSz == 0 || segSz >= outSz || + curSz >= outSz - segSz - (curSz != 1 ? 1 : 0)) { return WS_INVALID_PATH_E; } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 9d4671834..2d9f81182 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -6634,6 +6634,14 @@ static int wolfSSH_SFTP_DoStatus(WOLFSSH* ssh, word32 reqId, return WS_FATAL_ERROR; } + /* status is a small enumerated value (0..WOLFSSH_FTP_UNSUPPORTED). An + * out-of-range value from a malicious or broken server must not be + * returned as a negative int, where it would alias an internal WS_* + * error code and bypass the WOLFSSH_FTP_* classification in callers. */ + if (status > (word32)WOLFSSH_FTP_UNSUPPORTED) { + status = WOLFSSH_FTP_FAILURE; + } + /* read error message */ if (GetStringRef(&sz, &str, buf, maxIdx, &localIdx) != WS_SUCCESS) { return WS_FATAL_ERROR;