diff --git a/src/ssh.c b/src/ssh.c index 9c547c19..8c4b21ac 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -822,8 +822,8 @@ int wolfSSH_accept(WOLFSSH* ssh) const char* cmd = wolfSSH_GetSessionCommand(ssh); if (cmd != NULL && WOLFSSH_SESSION_SUBSYSTEM == wolfSSH_GetSessionType(ssh) - && ssh->channelList->commandSz == - (word32)WSTRLEN("sftp") + && wolfSSH_GetSessionCommandSz(ssh) + == (word32)WSTRLEN("sftp") && (WSTRCMP(cmd, "sftp") == 0)) { ssh->acceptState = ACCEPT_INIT_SFTP; return wolfSSH_SFTP_accept(ssh); @@ -4555,12 +4555,29 @@ WS_SessionType wolfSSH_GetSessionType(const WOLFSSH* ssh) const char* wolfSSH_GetSessionCommand(const WOLFSSH* ssh) { + const char* cmd = NULL; + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetSessionCommand()"); - if (ssh && ssh->channelList) - return ssh->channelList->command; + if (ssh) { + cmd = wolfSSH_ChannelGetSessionCommand(ssh->channelList); + } - return NULL; + return cmd; +} + + +word32 wolfSSH_GetSessionCommandSz(const WOLFSSH* ssh) +{ + word32 commandSz = 0; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetSessionCommandSz()"); + + if (ssh) { + commandSz = wolfSSH_ChannelGetSessionCommandSz(ssh->channelList); + } + + return commandSz; } @@ -5714,7 +5731,7 @@ const char* wolfSSH_ChannelGetSessionCommand(const WOLFSSH_CHANNEL* channel) { const char* cmd = NULL; - WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetCommand()"); + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetSessionCommand()"); if (channel) { cmd = channel->command; @@ -5724,6 +5741,20 @@ const char* wolfSSH_ChannelGetSessionCommand(const WOLFSSH_CHANNEL* channel) } +word32 wolfSSH_ChannelGetSessionCommandSz(const WOLFSSH_CHANNEL* channel) +{ + word32 commandSz = 0; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelGetSessionCommandSz()"); + + if (channel) { + commandSz = channel->commandSz; + } + + return commandSz; +} + + int wolfSSH_CTX_SetChannelOpenCb(WOLFSSH_CTX* ctx, WS_CallbackChannelOpen cb) { int ret = WS_SSH_CTX_NULL_E; diff --git a/tests/regress.c b/tests/regress.c index 545671ed..830b3d56 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3906,6 +3906,102 @@ static void TestChannelReqSubsysCallbackRuns(void) WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); } +/* What a length-aware session request callback saw. */ +static word32 sessionReqCbCommandSz; +static word32 sessionReqCbCommandStrLen; + +static int LengthRecordingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + const char* command; + + (void)ctx; + + sessionReqCbCalls++; + sessionReqCbCommandSz = wolfSSH_ChannelGetSessionCommandSz(channel); + command = wolfSSH_ChannelGetSessionCommand(channel); + sessionReqCbCommandStrLen = (command == NULL) ? + 0 : (word32)WSTRLEN(command); + + return 0; +} + +/* Drives one session request carrying a command that the C string alone + * cannot describe, and checks what the callback could see of it. */ +static void CheckSessionReqCbSeesCommandSz(const char* type, + const byte* command, word32 commandSz, word32 expectStrLen) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte payload[128]; + byte in[128]; + word32 idx = 0; + word32 inSz; + + sessionReqCbCalls = 0; + sessionReqCbCommandSz = 0; + sessionReqCbCommandStrLen = 0; + + InitChannelOpenHarness(&harness, NULL, 0); + if (WSTRCMP(type, "exec") == 0) { + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness.ctx, + LengthRecordingSessionReqCb), WS_SUCCESS); + } + else { + AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx, + LengthRecordingSessionReqCb), WS_SUCCESS); + } + + channel = SeedUnconfirmedChannel(&harness); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + /* Built here rather than with BuildChannelStringRequestPacket(): that + * takes the command as a C string, which cannot carry the NUL. */ + idx = AppendUint32(payload, sizeof(payload), idx, channel->channel); + idx = AppendString(payload, sizeof(payload), idx, type); + idx = AppendByte(payload, sizeof(payload), idx, 1); + idx = AppendUint32(payload, sizeof(payload), idx, commandSz); + idx = AppendData(payload, sizeof(payload), idx, command, commandSz); + inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in)); + RepointHarnessInput(&harness, in, inSz); + + AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS); + AssertIntEQ(sessionReqCbCalls, 1); + AssertIntEQ(sessionReqCbCommandSz, commandSz); + AssertIntEQ(sessionReqCbCommandStrLen, expectStrLen); + + /* The session-wide accessor reports the same channel's command. */ + AssertIntEQ(wolfSSH_GetSessionCommandSz(harness.ssh), commandSz); + + FreeChannelOpenHarness(&harness); +} + +/* An application vetting a command in its callback needs the wire length. + * The string it is handed stops at an embedded NUL, so "sftp\0evil" reads + * there as "sftp" and passes a name check the whole name has to fail; the + * length is what tells the two apart. */ +static void TestSessionReqCallbackSeesCommandSz(void) +{ + static const byte nulCommand[] = { + 's', 'f', 't', 'p', 0, 'e', 'v', 'i', 'l' + }; + static const byte plainCommand[] = { 'l', 's' }; + + /* The control: with no NUL in it, length and C string agree, so the + * cases below are the NUL and not the accessor reporting anything it + * likes. */ + CheckSessionReqCbSeesCommandSz("exec", plainCommand, + (word32)sizeof(plainCommand), (word32)sizeof(plainCommand)); + CheckSessionReqCbSeesCommandSz("exec", nulCommand, + (word32)sizeof(nulCommand), 4); + CheckSessionReqCbSeesCommandSz("subsystem", nulCommand, + (word32)sizeof(nulCommand), 4); + + /* Nothing to report is zero, not a read through a NULL. */ + AssertIntEQ(wolfSSH_ChannelGetSessionCommandSz(NULL), 0); + AssertIntEQ(wolfSSH_GetSessionCommandSz(NULL), 0); +} + /* A request callback owns its channel and may close it. The grant is * recorded after the callback returns, so it has to find the channel * again: wolfSSH_ChannelFree() frees it, and writing through the old @@ -14462,6 +14558,7 @@ int main(int argc, char** argv) TestChannelCloseCallbackReturnIgnored(); TestChannelReqExecCallbackRuns(); TestChannelReqSubsysCallbackRuns(); + TestSessionReqCallbackSeesCommandSz(); TestSessionReqCallbackMayFreeChannel(); TestAppChannelsAcceptKeepsStopWithPendingOutput(); #ifdef WOLFSSH_SFTP diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 005bea77..8a4ae0ca 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -436,6 +436,8 @@ WOLFSSH_API WS_SessionType wolfSSH_ChannelGetSessionType( const WOLFSSH_CHANNEL* channel); WOLFSSH_API const char* wolfSSH_ChannelGetSessionCommand( const WOLFSSH_CHANNEL* channel); +WOLFSSH_API word32 wolfSSH_ChannelGetSessionCommandSz( + const WOLFSSH_CHANNEL* channel); WOLFSSH_API int wolfSSH_ChannelIsPty(const WOLFSSH_CHANNEL* channel); /* Channel callbacks */ @@ -895,6 +897,7 @@ WOLFSSH_API int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, WOLFSSH_API int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd); WOLFSSH_API WS_SessionType wolfSSH_GetSessionType(const WOLFSSH* ssh); WOLFSSH_API const char* wolfSSH_GetSessionCommand(const WOLFSSH* ssh); +WOLFSSH_API word32 wolfSSH_GetSessionCommandSz(const WOLFSSH* ssh); WOLFSSH_API int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz); WOLFSSH_API int wolfSSH_ChangeTerminalSize(WOLFSSH* ssh, word32 columns,