Skip to content

Commit 978ef5c

Browse files
ejohnstownpadelsbach
authored andcommitted
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 aef3e86 commit 978ef5c

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
@@ -6722,18 +6722,17 @@ static int DoUnimplemented(WOLFSSH* ssh,
67226722
{
67236723
word32 seq;
67246724
word32 begin = *idx;
6725+
int ret;
67256726

67266727
WOLFSSH_UNUSED(ssh);
6727-
WOLFSSH_UNUSED(len);
6728-
6729-
ato32(buf + begin, &seq);
6730-
begin += UINT32_SZ;
6731-
6732-
WLOG(WS_LOG_DEBUG, "UNIMPLEMENTED: seq %u", seq);
67336728

6734-
*idx = begin;
6729+
ret = GetUint32(&seq, buf, len, &begin);
6730+
if (ret == WS_SUCCESS) {
6731+
*idx = begin;
6732+
WLOG(WS_LOG_DEBUG, "UNIMPLEMENTED: seq %u", seq);
6733+
}
67356734

6736-
return WS_SUCCESS;
6735+
return ret;
67376736
}
67386737

67396738

@@ -6742,57 +6741,67 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
67426741
word32 reason;
67436742
const char* reasonStr = NULL;
67446743
word32 begin = *idx;
6744+
int ret;
67456745

6746-
WOLFSSH_UNUSED(len);
67476746
WOLFSSH_UNUSED(reasonStr);
67486747

6749-
ato32(buf + begin, &reason);
6750-
begin += UINT32_SZ;
6748+
ret = GetUint32(&reason, buf, len, &begin);
6749+
if (ret == WS_SUCCESS) {
6750+
/* Skip the description text. */
6751+
ret = GetSkip(buf, len, &begin);
6752+
}
6753+
if (ret == WS_SUCCESS) {
6754+
/* Skip the language identifier. */
6755+
ret = GetSkip(buf, len, &begin);
6756+
}
6757+
6758+
if (ret == WS_SUCCESS) {
6759+
*idx = begin;
6760+
ssh->error = WS_DISCONNECT;
6761+
ret = WS_DISCONNECT;
67516762

67526763
#ifdef NO_WOLFSSH_STRINGS
6753-
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u)", reason);
6764+
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u)", reason);
67546765
#elif defined(DEBUG_WOLFSSH)
6755-
switch (reason) {
6756-
case WOLFSSH_DISCONNECT_HOST_NOT_ALLOWED_TO_CONNECT:
6757-
reasonStr = "host not allowed to connect"; break;
6758-
case WOLFSSH_DISCONNECT_PROTOCOL_ERROR:
6759-
reasonStr = "protocol error"; break;
6760-
case WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED:
6761-
reasonStr = "key exchange failed"; break;
6762-
case WOLFSSH_DISCONNECT_RESERVED:
6763-
reasonStr = "reserved"; break;
6764-
case WOLFSSH_DISCONNECT_MAC_ERROR:
6765-
reasonStr = "mac error"; break;
6766-
case WOLFSSH_DISCONNECT_COMPRESSION_ERROR:
6767-
reasonStr = "compression error"; break;
6768-
case WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE:
6769-
reasonStr = "service not available"; break;
6770-
case WOLFSSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED:
6771-
reasonStr = "protocol version not supported"; break;
6772-
case WOLFSSH_DISCONNECT_HOST_KEY_NOT_VERIFIABLE:
6773-
reasonStr = "host key not verifiable"; break;
6774-
case WOLFSSH_DISCONNECT_CONNECTION_LOST:
6775-
reasonStr = "connection lost"; break;
6776-
case WOLFSSH_DISCONNECT_BY_APPLICATION:
6777-
reasonStr = "disconnect by application"; break;
6778-
case WOLFSSH_DISCONNECT_TOO_MANY_CONNECTIONS:
6779-
reasonStr = "too many connections"; break;
6780-
case WOLFSSH_DISCONNECT_AUTH_CANCELLED_BY_USER:
6781-
reasonStr = "auth cancelled by user"; break;
6782-
case WOLFSSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE:
6783-
reasonStr = "no more auth methods available"; break;
6784-
case WOLFSSH_DISCONNECT_ILLEGAL_USER_NAME:
6785-
reasonStr = "illegal user name"; break;
6786-
default:
6787-
reasonStr = "unknown reason";
6788-
}
6789-
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u) %s", reason, reasonStr);
6766+
switch (reason) {
6767+
case WOLFSSH_DISCONNECT_HOST_NOT_ALLOWED_TO_CONNECT:
6768+
reasonStr = "host not allowed to connect"; break;
6769+
case WOLFSSH_DISCONNECT_PROTOCOL_ERROR:
6770+
reasonStr = "protocol error"; break;
6771+
case WOLFSSH_DISCONNECT_KEY_EXCHANGE_FAILED:
6772+
reasonStr = "key exchange failed"; break;
6773+
case WOLFSSH_DISCONNECT_RESERVED:
6774+
reasonStr = "reserved"; break;
6775+
case WOLFSSH_DISCONNECT_MAC_ERROR:
6776+
reasonStr = "mac error"; break;
6777+
case WOLFSSH_DISCONNECT_COMPRESSION_ERROR:
6778+
reasonStr = "compression error"; break;
6779+
case WOLFSSH_DISCONNECT_SERVICE_NOT_AVAILABLE:
6780+
reasonStr = "service not available"; break;
6781+
case WOLFSSH_DISCONNECT_PROTOCOL_VERSION_NOT_SUPPORTED:
6782+
reasonStr = "protocol version not supported"; break;
6783+
case WOLFSSH_DISCONNECT_HOST_KEY_NOT_VERIFIABLE:
6784+
reasonStr = "host key not verifiable"; break;
6785+
case WOLFSSH_DISCONNECT_CONNECTION_LOST:
6786+
reasonStr = "connection lost"; break;
6787+
case WOLFSSH_DISCONNECT_BY_APPLICATION:
6788+
reasonStr = "disconnect by application"; break;
6789+
case WOLFSSH_DISCONNECT_TOO_MANY_CONNECTIONS:
6790+
reasonStr = "too many connections"; break;
6791+
case WOLFSSH_DISCONNECT_AUTH_CANCELLED_BY_USER:
6792+
reasonStr = "auth cancelled by user"; break;
6793+
case WOLFSSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE:
6794+
reasonStr = "no more auth methods available"; break;
6795+
case WOLFSSH_DISCONNECT_ILLEGAL_USER_NAME:
6796+
reasonStr = "illegal user name"; break;
6797+
default:
6798+
reasonStr = "unknown reason";
6799+
}
6800+
WLOG(WS_LOG_DEBUG, "DISCONNECT: (%u) %s", reason, reasonStr);
67906801
#endif
6802+
}
67916803

6792-
*idx = begin;
6793-
6794-
ssh->error = WS_DISCONNECT;
6795-
return WS_DISCONNECT;
6804+
return ret;
67966805
}
67976806

67986807

0 commit comments

Comments
 (0)