ssh: match channel request types exactly

- add ChannelRequestIs() comparing the type length as well
- use it for all nine handled request types, so truncated,
  empty and NUL-padded types are rejected
- cover "sh", "sub", empty and "shell\0AAAA" in unit tests

Issue: F-7217
pull/1154/head
John Safranek 2026-08-04 15:34:56 -07:00 committed by JacobBarthelmeh
parent 7c4b1d3549
commit 319180beff
2 changed files with 95 additions and 9 deletions

View File

@ -10998,6 +10998,15 @@ int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd)
#endif /* !NO_TERMIOS && WOLFSSH_TERM */
/* Exact match on a channel request type, as NameToIdType() does for names. */
static int ChannelRequestIs(const char* type, word32 typeSz, const char* name)
{
word32 nameSz = (word32)WSTRLEN(name);
return (typeSz == nameSz) && (WSTRNCMP(type, name, nameSz) == 0);
}
static int DoChannelRequest(WOLFSSH* ssh,
byte* buf, word32 len, word32* idx)
{
@ -11036,7 +11045,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
WLOG(WS_LOG_DEBUG, " type = %s", type);
WLOG(WS_LOG_DEBUG, " wantReply = %u", wantReply);
if (WSTRNCMP(type, "env", typeSz) == 0) {
if (ChannelRequestIs(type, typeSz, "env")) {
char name[WOLFSSH_MAX_NAMESZ];
word32 nameSz;
char value[32];
@ -11052,14 +11061,14 @@ static int DoChannelRequest(WOLFSSH* ssh,
WLOG(WS_LOG_DEBUG, " %s = %s", name, value);
}
else if (WSTRNCMP(type, "shell", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "shell")) {
channel->sessionType = WOLFSSH_SESSION_SHELL;
if (ssh->ctx->channelReqShellCb) {
rej = ssh->ctx->channelReqShellCb(channel, ssh->channelReqCtx);
}
ssh->clientState = CLIENT_DONE;
}
else if (WSTRNCMP(type, "exec", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "exec")) {
ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
buf, len, &begin);
channel->sessionType = WOLFSSH_SESSION_EXEC;
@ -11070,7 +11079,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
WLOG(WS_LOG_DEBUG, " command = %s", channel->command);
}
else if (WSTRNCMP(type, "subsystem", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "subsystem")) {
ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
buf, len, &begin);
channel->sessionType = WOLFSSH_SESSION_SUBSYSTEM;
@ -11082,7 +11091,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command);
}
#ifdef WOLFSSH_TERM
else if (WSTRNCMP(type, "pty-req", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "pty-req")) {
char term[32];
word32 termSz;
@ -11120,7 +11129,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
}
#endif /* WOLFSSH_TERM */
#if defined(WOLFSSH_SHELL) && defined(WOLFSSH_TERM)
else if (WSTRNCMP(type, "window-change", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "window-change")) {
word32 widthChar, heightRows, widthPixels, heightPixels;
wantReply = 0; /* RFC 4254 sec 6.7: no reply for window-change */
@ -11152,12 +11161,12 @@ static int DoChannelRequest(WOLFSSH* ssh,
}
#endif /* WOLFSSH_SHELL && WOLFSSH_TERM */
#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL)
else if (WSTRNCMP(type, "exit-status", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "exit-status")) {
wantReply = 0; /* RFC 4254 sec 6.10: no reply for exit-status */
ret = GetUint32(&ssh->exitStatus, buf, len, &begin);
WLOG(WS_LOG_AGENT, "Got exit status %u.", ssh->exitStatus);
}
else if (WSTRNCMP(type, "exit-signal", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "exit-signal")) {
char sig[WOLFSSH_MAX_NAMESZ];
word32 sigSz;
byte coreDumped;
@ -11188,7 +11197,7 @@ static int DoChannelRequest(WOLFSSH* ssh,
}
#endif /* WOLFSSH_TERM or WOLFSSH_SHELL */
#ifdef WOLFSSH_AGENT
else if (WSTRNCMP(type, "auth-agent-req@openssh.com", typeSz) == 0) {
else if (ChannelRequestIs(type, typeSz, "auth-agent-req@openssh.com")) {
WLOG(WS_LOG_AGENT, " ssh-agent");
if (ssh->ctx->agentCb != NULL)
ssh->useAgent = 1;

View File

@ -5961,6 +5961,83 @@ static int test_DoChannelRequest(void)
goto done;
}
/* A request type must match a handled name exactly. Truncated,
* empty, and NUL-padded types have to be rejected. These run before
* the positive cases below, which set the channel session type. */
{
static const byte payShellPrefix[] = {
0x00,0x00,0x00,0x00, /* channelId = 0 */
0x00,0x00,0x00,0x02, /* typeSz = 2 */
0x73,0x68, /* "sh" */
0x01 /* wantReply = 1 */
};
static const byte paySubPrefix[] = {
0x00,0x00,0x00,0x00, /* channelId = 0 */
0x00,0x00,0x00,0x03, /* typeSz = 3 */
0x73,0x75,0x62, /* "sub" */
0x01 /* wantReply = 1 */
};
static const byte payEmptyType[] = {
0x00,0x00,0x00,0x00, /* channelId = 0 */
0x00,0x00,0x00,0x00, /* typeSz = 0 */
0x01, /* wantReply = 1 */
0x00,0x00,0x00,0x00, /* env name = "" */
0x00,0x00,0x00,0x00 /* env value = "" */
};
static const byte payShellNul[] = {
0x00,0x00,0x00,0x00, /* channelId = 0 */
0x00,0x00,0x00,0x0A, /* typeSz = 10 */
0x73,0x68,0x65,0x6C,0x6C, /* "shell" */
0x00,0x41,0x41,0x41,0x41, /* "\0AAAA" */
0x01 /* wantReply = 1 */
};
struct { const char* label; const byte* buf; word32 sz; } badCases[] = {
{ "sh", payShellPrefix, (word32)sizeof(payShellPrefix) },
{ "sub", paySubPrefix, (word32)sizeof(paySubPrefix) },
{ "empty", payEmptyType, (word32)sizeof(payEmptyType) },
{ "shell-nul", payShellNul, (word32)sizeof(payShellNul) },
};
int b;
for (b = 0; b < (int)(sizeof(badCases)/sizeof(badCases[0])); b++) {
word32 idxBad = 0;
int retBad, capMsgId;
s_chanReqCaptureSz = 0;
WMEMSET(s_chanReqCapture, 0, sizeof(s_chanReqCapture));
retBad = wolfSSH_TestDoChannelRequest(ssh, (byte*)badCases[b].buf,
badCases[b].sz, &idxBad);
if (retBad != WS_SUCCESS) {
printf("DoChannelRequest[%s]: ret=%d, expected=%d\n",
badCases[b].label, retBad, WS_SUCCESS);
result = -460 - b;
goto done;
}
capMsgId = CaptureMsgId(s_chanReqCapture, s_chanReqCaptureSz);
if (capMsgId != (int)MSGID_CHANNEL_FAILURE) {
printf("DoChannelRequest[%s]: msg_id=0x%02x, expected=0x%02x\n",
badCases[b].label, capMsgId, MSGID_CHANNEL_FAILURE);
result = -470 - b;
goto done;
}
if (ch->sessionType == WOLFSSH_SESSION_SHELL) {
printf("DoChannelRequest[%s]: session type changed\n",
badCases[b].label);
result = -480 - b;
goto done;
}
if (ssh->clientState == CLIENT_DONE) {
printf("DoChannelRequest[%s]: client state changed\n",
badCases[b].label);
result = -490 - b;
goto done;
}
}
}
for (i = 0; i < (int)(sizeof(cases) / sizeof(cases[0])); i++) {
word32 idx = 0;
int ret;