Skip to content

Commit 6638c01

Browse files
committed
DoServiceRequest Missing Bounds Check
Replace the original message parsing functions with the GetString() function, which does better bounds checking. Affected functions: DoServiceRequest, DoServiceAccept. Issue: F-524, F-525
1 parent 9cb60cd commit 6638c01

File tree

1 file changed

+16
-36
lines changed

1 file changed

+16
-36
lines changed

src/internal.c

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6523,56 +6523,36 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
65236523
static int DoServiceRequest(WOLFSSH* ssh,
65246524
byte* buf, word32 len, word32* idx)
65256525
{
6526-
word32 begin = *idx;
6527-
word32 nameSz;
6528-
char serviceName[WOLFSSH_MAX_NAMESZ];
6529-
6530-
WOLFSSH_UNUSED(len);
6526+
char name[WOLFSSH_MAX_NAMESZ+1];
6527+
word32 nameSz = sizeof(name);
6528+
int ret;
65316529

6532-
ato32(buf + begin, &nameSz);
6533-
begin += LENGTH_SZ;
6530+
ret = GetString(name, &nameSz, buf, len, idx);
65346531

6535-
if (begin + nameSz > len || nameSz >= WOLFSSH_MAX_NAMESZ) {
6536-
return WS_BUFFER_E;
6532+
if (ret == WS_SUCCESS) {
6533+
WLOG(WS_LOG_DEBUG, "Requesting service: %s", name);
6534+
ssh->clientState = CLIENT_USERAUTH_REQUEST_DONE;
65376535
}
65386536

6539-
WMEMCPY(serviceName, buf + begin, nameSz);
6540-
begin += nameSz;
6541-
serviceName[nameSz] = 0;
6542-
6543-
*idx = begin;
6544-
6545-
WLOG(WS_LOG_DEBUG, "Requesting service: %s", serviceName);
6546-
ssh->clientState = CLIENT_USERAUTH_REQUEST_DONE;
6547-
6548-
return WS_SUCCESS;
6537+
return ret;
65496538
}
65506539

65516540

65526541
static int DoServiceAccept(WOLFSSH* ssh,
65536542
byte* buf, word32 len, word32* idx)
65546543
{
6555-
word32 begin = *idx;
6556-
word32 nameSz;
6557-
char serviceName[WOLFSSH_MAX_NAMESZ];
6544+
char name[WOLFSSH_MAX_NAMESZ+1];
6545+
word32 nameSz = sizeof(name);
6546+
int ret;
65586547

6559-
ato32(buf + begin, &nameSz);
6560-
begin += LENGTH_SZ;
6548+
ret = GetString(name, &nameSz, buf, len, idx);
65616549

6562-
if (begin + nameSz > len || nameSz >= WOLFSSH_MAX_NAMESZ) {
6563-
return WS_BUFFER_E;
6550+
if (ret == WS_SUCCESS) {
6551+
WLOG(WS_LOG_DEBUG, "Accepted service: %s", name);
6552+
ssh->serverState = SERVER_USERAUTH_REQUEST_DONE;
65646553
}
65656554

6566-
WMEMCPY(serviceName, buf + begin, nameSz);
6567-
begin += nameSz;
6568-
serviceName[nameSz] = 0;
6569-
6570-
*idx = begin;
6571-
6572-
WLOG(WS_LOG_DEBUG, "Accepted service: %s", serviceName);
6573-
ssh->serverState = SERVER_USERAUTH_REQUEST_DONE;
6574-
6575-
return WS_SUCCESS;
6555+
return ret;
65766556
}
65776557

65786558

0 commit comments

Comments
 (0)