diff --git a/src/internal.c b/src/internal.c index 895b80eb..91a8571b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -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; diff --git a/tests/unit.c b/tests/unit.c index f6c3ded1..736e843b 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -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;