mirror of https://github.com/wolfSSL/wolfssh.git
ssh: expose the session command length
An application vetting an exec or subsystem request in its channel request callback is handed the command as a C string, which stops at an embedded NUL. wolfSSH_ChannelGetSessionCommandSz() and wolfSSH_GetSessionCommandSz() report the parsed wire length, so a callback can match a name whole the way DoChannelRequest() does. - both accessors report 0 for a NULL channel or session - wolfSSH_GetSessionCommand() defers to the channel accessor - the sftp divert in wolfSSH_accept() asks the accessor for the length - correct the trace name in wolfSSH_ChannelGetSessionCommand() - cover a callback seeing "sftp\0evil" through exec and subsystempull/1235/head
parent
44a348f01a
commit
9c8caea8b5
43
src/ssh.c
43
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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue