Skip to content

Commit a450317

Browse files
committed
DoDisconnect/DoUnimplemented: validate payload
- Replace raw ato32() with GetUint32() to check len. - Return WS_BUFFER_E on short payload instead of reading past buffer end. - DoDisconnect: parse the description and language identifier strings with GetSkip() so a truncated record returns WS_BUFFER_E instead of being treated as success. Issue: F-413
1 parent 204f4ff commit a450317

1 file changed

Lines changed: 60 additions & 51 deletions

File tree

src/internal.c

Lines changed: 60 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6667,18 +6667,17 @@ static int DoUnimplemented(WOLFSSH* ssh,
66676667
{
66686668
word32 seq;
66696669
word32 begin = *idx;
6670+
int ret;
66706671

66716672
WOLFSSH_UNUSED(ssh);
6672-
WOLFSSH_UNUSED(len);
6673-
6674-
ato32(buf + begin, &seq);
6675-
begin += UINT32_SZ;
6676-
6677-
WLOG(WS_LOG_DEBUG, "UNIMPLEMENTED: seq %u", seq);
66786673

6679-
*idx = begin;
6674+
ret = GetUint32(&seq, buf, len, &begin);
6675+
if (ret == WS_SUCCESS) {
6676+
*idx = begin;
6677+
WLOG(WS_LOG_DEBUG, "UNIMPLEMENTED: seq %u", seq);
6678+
}
66806679

6681-
return WS_SUCCESS;
6680+
return ret;
66826681
}
66836682

66846683

@@ -6687,57 +6686,67 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
66876686
word32 reason;
66886687
const char* reasonStr = NULL;
66896688
word32 begin = *idx;
6689+
int ret;
66906690

6691-
WOLFSSH_UNUSED(len);
66926691
WOLFSSH_UNUSED(reasonStr);
66936692

6694-
ato32(buf + begin, &reason);
6695-
begin += UINT32_SZ;
6693+
ret = GetUint32(&reason, buf, len, &begin);
6694+
if (ret == WS_SUCCESS) {
6695+
/* Skip the description text. */
6696+
ret = GetSkip(buf, len, &begin);
6697+
}
6698+
if (ret == WS_SUCCESS) {
6699+
/* Skip the language identifier. */
6700+
ret = GetSkip(buf, len, &begin);
6701+
}
6702+
6703+
if (ret == WS_SUCCESS) {
6704+
*idx = begin;
6705+
ssh->error = WS_DISCONNECT;
6706+
ret = WS_DISCONNECT;
66966707

66976708
#ifdef NO_WOLFSSH_STRINGS
6698-
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u)", reason);
6709+
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u)", reason);
66996710
#elif defined(DEBUG_WOLFSSH)
6700-
switch (reason) {
6701-
case WOLFSSH_DISCONNECT_HOST_NOT_ALLOWED_TO_CONNECT:
6702-
reasonStr = "host not allowed to connect"; break;
6703-
case WOLFSSH_DISCONNECT_PROTOCOL_ERROR:
6704-
reasonStr = "protocol error"; break;
6705-
case WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED:
6706-
reasonStr = "key exchange failed"; break;
6707-
case WOLFSSH_DISCONNECT_RESERVED:
6708-
reasonStr = "reserved"; break;
6709-
case WOLFSSH_DISCONNECT_MAC_ERROR:
6710-
reasonStr = "mac error"; break;
6711-
case WOLFSSH_DISCONNECT_COMPRESSION_ERROR:
6712-
reasonStr = "compression error"; break;
6713-
case WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE:
6714-
reasonStr = "service not available"; break;
6715-
case WOLFSSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED:
6716-
reasonStr = "protocol version not supported"; break;
6717-
case WOLFSSH_DISCONNECT_HOST_KEY_NOT_VERIFIABLE:
6718-
reasonStr = "host key not verifiable"; break;
6719-
case WOLFSSH_DISCONNECT_CONNECTION_LOST:
6720-
reasonStr = "connection lost"; break;
6721-
case WOLFSSH_DISCONNECT_BY_APPLICATION:
6722-
reasonStr = "disconnect by application"; break;
6723-
case WOLFSSH_DISCONNECT_TOO_MANY_CONNECTIONS:
6724-
reasonStr = "too many connections"; break;
6725-
case WOLFSSH_DISCONNECT_AUTH_CANCELLED_BY_USER:
6726-
reasonStr = "auth cancelled by user"; break;
6727-
case WOLFSSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE:
6728-
reasonStr = "no more auth methods available"; break;
6729-
case WOLFSSH_DISCONNECT_ILLEGAL_USER_NAME:
6730-
reasonStr = "illegal user name"; break;
6731-
default:
6732-
reasonStr = "unknown reason";
6733-
}
6734-
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u) %s", reason, reasonStr);
6711+
switch (reason) {
6712+
case WOLFSSH_DISCONNECT_HOST_NOT_ALLOWED_TO_CONNECT:
6713+
reasonStr = "host not allowed to connect"; break;
6714+
case WOLFSSH_DISCONNECT_PROTOCOL_ERROR:
6715+
reasonStr = "protocol error"; break;
6716+
case WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED:
6717+
reasonStr = "key exchange failed"; break;
6718+
case WOLFSSH_DISCONNECT_RESERVED:
6719+
reasonStr = "reserved"; break;
6720+
case WOLFSSH_DISCONNECT_MAC_ERROR:
6721+
reasonStr = "mac error"; break;
6722+
case WOLFSSH_DISCONNECT_COMPRESSION_ERROR:
6723+
reasonStr = "compression error"; break;
6724+
case WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE:
6725+
reasonStr = "service not available"; break;
6726+
case WOLFSSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED:
6727+
reasonStr = "protocol version not supported"; break;
6728+
case WOLFSSH_DISCONNECT_HOST_KEY_NOT_VERIFIABLE:
6729+
reasonStr = "host key not verifiable"; break;
6730+
case WOLFSSH_DISCONNECT_CONNECTION_LOST:
6731+
reasonStr = "connection lost"; break;
6732+
case WOLFSSH_DISCONNECT_BY_APPLICATION:
6733+
reasonStr = "disconnect by application"; break;
6734+
case WOLFSSH_DISCONNECT_TOO_MANY_CONNECTIONS:
6735+
reasonStr = "too many connections"; break;
6736+
case WOLFSSH_DISCONNECT_AUTH_CANCELLED_BY_USER:
6737+
reasonStr = "auth cancelled by user"; break;
6738+
case WOLFSSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE:
6739+
reasonStr = "no more auth methods available"; break;
6740+
case WOLFSSH_DISCONNECT_ILLEGAL_USER_NAME:
6741+
reasonStr = "illegal user name"; break;
6742+
default:
6743+
reasonStr = "unknown reason";
6744+
}
6745+
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u) %s", reason, reasonStr);
67356746
#endif
6747+
}
67366748

6737-
*idx = begin;
6738-
6739-
ssh->error = WS_DISCONNECT;
6740-
return WS_DISCONNECT;
6749+
return ret;
67416750
}
67426751

67436752

0 commit comments

Comments
 (0)