WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

scp: match the scp command as a whole token 

wolfSSH_ChannelCommandIsScp() reports whether a channel's session
command starts an SCP transfer, taking "scp" only as its own token. The
divert in wolfSSH_accept() and an application's exec callback both ask
it, so the two cannot disagree about what the SCP server is handed.

- an exec of "scpbackup foo" runs as an ordinary exec
- a command carrying a NUL within the recorded command size is not an
  SCP command; ParseScpCommand() walks a C string, so a NUL would drop
  whatever follows it
- wolfsshd's session callback asks through the same helper
pull/1253/head
John Safranek 2026-09-15 19:57:06 -07:00 committed by Paul Adelsbach
parent 74a296d9c2
commit fd4d462510
6 changed files with 116 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
apps/wolfsshd/wolfsshd.c
View File

@ -502,18 +502,11 @@ static int SessionRequestCb(WOLFSSH_CHANNEL* channel, void* vCtx)
break;
}
#ifdef WOLFSSH_SCP
{
word32 cmdSz = wolfSSH_ChannelGetSessionCommandSz(channel);
/* "scp" must stand as its own token; a prefix match
* grants "scpbackup", and a NUL makes it another. */
if (cmdSz >= (word32)WSTRLEN("scp")
&& WSTRNCMP(cmd, "scp", 3) == 0
&& (cmdSz == (word32)WSTRLEN("scp")
|| cmd[3] == ' ')) {
rej = 0;
break;
}
/* Shared with the SCP divert in wolfSSH_accept(), so the two
* cannot disagree about what starts a transfer. */
if (wolfSSH_ChannelCommandIsScp(channel) == 1) {
rej = 0;
break;
}
#endif
#ifdef WOLFSSH_SHELL

3
src/ssh.c
View File

@ -831,7 +831,8 @@ int wolfSSH_accept(WOLFSSH* ssh)
#ifdef WOLFSSH_SCP
if (ssh->channelList != NULL
&& ssh->channelList->sessionGranted
&& ChannelCommandIsScp(ssh)) {
&& wolfSSH_ChannelCommandIsScp(ssh->channelList)
== 1) {
ssh->acceptState = ACCEPT_INIT_SCP_TRANSFER;
WLOG(WS_LOG_DEBUG, acceptState, "ACCEPT_INIT_SCP_TRANSFER");
return WS_SCP_INIT;

30
src/wolfscp.c
View File

@ -1040,20 +1040,34 @@ WOLFSSH_API int wolfSSH_SetScpErrorMsg(WOLFSSH* ssh, const char* message)
return ret;
}
/* Determine if channel command sent in initial negotiation is scp.
* Return 1 if yes, 0 if no */
int ChannelCommandIsScp(WOLFSSH* ssh)
/* Determine if the channel's session command is scp. See wolfscp.h for
* the contract; "scp" must stand as its own token. */
int wolfSSH_ChannelCommandIsScp(const WOLFSSH_CHANNEL* channel)
{
const char* cmd;
word32 cmdSz;
word32 scpSz = (word32)WSTRLEN("scp");
word32 i;
int ret = 0;
if (ssh == NULL)
if (channel == NULL)
return WS_BAD_ARGUMENT;
cmd = wolfSSH_GetSessionCommand(ssh);
if (cmd != NULL && WSTRLEN(cmd) >= 3) {
if (WSTRNCMP(cmd, "scp", 3) == 0)
ret = 1;
cmd = wolfSSH_ChannelGetSessionCommand(channel);
cmdSz = wolfSSH_ChannelGetSessionCommandSz(channel);
if (cmd != NULL && cmdSz >= scpSz
&& WSTRNCMP(cmd, "scp", scpSz) == 0
&& (cmdSz == scpSz || cmd[scpSz] == ' ')) {
ret = 1;
}
/* The parse that follows is a C string walk, so a NUL inside the
* command would drop the rest of it. Refuse rather than transfer
* something other than what was asked for. */
for (i = 0; ret == 1 && i < cmdSz; i++) {
if (cmd[i] == '\0')
ret = 0;
}
return ret;

73
tests/regress.c
View File

@ -5788,6 +5788,78 @@ static void TestAcceptDivertNeedsScpGrantAlone(void)
FreeChannelOpenHarness(&harness);
}
/* The SCP divert reads only the command, so "scp" has to stand as its own
* token: a bare prefix match takes "scpbackup" into the built-in SCP
* server. A NUL anywhere in the command hides what follows it from the
* parse that serves the transfer, whether it sits where the space belongs
* or past a good token boundary. wolfsshd's exec callback asks the same
* question with the same helper, so the two cannot answer it
* differently. */
static void TestAcceptDivertMatchesScpCommandToken(void)
{
static const struct {
const char* cmd;
word32 cmdSz;
int divert;
} cases[] = {
{ "scp -f /tmp/a", 13, 1 }, /* what the real client sends */
{ "scp", 3, 1 }, /* the token alone */
{ "scpbackup foo", 13, 0 }, /* the token is only a prefix */
{ "scp\0 -f /tmp/a", 14, 0 }, /* a NUL where the space belongs */
{ "scp -t /d\0x", 11, 0 }, /* a NUL hiding a tail past the token */
{ "scp -t /d\0", 10, 0 }, /* a NUL counted at the end */
{ "sc", 2, 0 }, /* shorter than the token */
{ "ls", 2, 0 }
};
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
byte payload[128];
byte in[128];
word32 idx;
word32 inSz;
word32 i;
AssertIntEQ(wolfSSH_ChannelCommandIsScp(NULL), WS_BAD_ARGUMENT);
for (i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
InitChannelOpenHarness(&harness, NULL, 0);
channel = SeedUnconfirmedChannel(&harness);
AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS);
channel->openConfirmed = 1;
idx = 0;
idx = AppendUint32(payload, sizeof(payload), idx, channel->channel);
idx = AppendString(payload, sizeof(payload), idx, "exec");
idx = AppendByte(payload, sizeof(payload), idx, 1);
idx = AppendUint32(payload, sizeof(payload), idx, cases[i].cmdSz);
idx = AppendData(payload, sizeof(payload), idx,
(const byte*)cases[i].cmd, cases[i].cmdSz);
inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in));
RepointHarnessInput(&harness, in, inSz);
/* Neither app-channels nor a callback, so the request is granted
* and the session is the one wolfSSH_accept() goes on to serve. */
AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS);
AssertIntEQ(channel->commandSz, cases[i].cmdSz);
AssertIntEQ(wolfSSH_ChannelCommandIsScp(channel), cases[i].divert);
RepointHarnessInput(&harness, NULL, 0);
harness.ssh->acceptState = ACCEPT_SERVER_CHANNEL_ACCEPT_SENT;
if (cases[i].divert) {
AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SCP_INIT);
AssertIntEQ(harness.ssh->acceptState, ACCEPT_INIT_SCP_TRANSFER);
}
else {
AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS);
AssertIntEQ(harness.ssh->acceptState,
ACCEPT_CLIENT_SESSION_ESTABLISHED);
}
FreeChannelOpenHarness(&harness);
}
}
#endif /* WOLFSSH_SCP */
/* A username change after the first userauth request must end the session. */
@ -15955,6 +16027,7 @@ int main(int argc, char** argv)
TestAcceptDivertNeedsSftpGrantAlone();
#endif
#ifdef WOLFSSH_SCP
TestAcceptDivertMatchesScpCommandToken();
TestAcceptDivertNeedsScpGrant();
TestAcceptDivertNeedsScpGrantAlone();
#endif

1
wolfssh/internal.h
View File

@ -2259,7 +2259,6 @@ enum WS_ScpDirection {
WOLFSSH_SCP_FROM
};
WOLFSSH_LOCAL int ChannelCommandIsScp(WOLFSSH* ssh);
WOLFSSH_LOCAL int DoScpRequest(WOLFSSH* ssh);
WOLFSSH_LOCAL int DoScpSink(WOLFSSH* ssh);
WOLFSSH_LOCAL int DoScpSource(WOLFSSH* ssh);

14
wolfssh/wolfscp.h
View File

@ -170,6 +170,20 @@ WOLFSSH_API int wolfSSH_SCP_from(WOLFSSH* ssh, const char* src,
* done; call it again on the same session until it completes. */
WOLFSSH_API int wolfSSH_SCP_accept(WOLFSSH* ssh);
/* Tells an SCP request from an ordinary exec, for use from an exec
* channel-request callback and by wolfSSH_accept() itself, so the two
* cannot disagree about what starts a transfer.
*
* Returns 1 when the channel's session command starts an SCP transfer, 0
* when it does not, and WS_BAD_ARGUMENT when channel is NULL. "scp" must
* stand as its own token: a bare prefix match would take "scpbackup" as a
* transfer. The test runs over the recorded command size rather than the
* string length, and a command carrying a NUL anywhere within that size
* is not an SCP command: the parse that serves the transfer walks a C
* string, so a NUL would silently drop whatever follows it. */
WOLFSSH_API int wolfSSH_ChannelCommandIsScp(
const WOLFSSH_CHANNEL* channel);
#ifdef __cplusplus
}