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

ssh, sftp: match the sftp subsystem name exactly 

The built-in SFTP server takes a session only when the subsystem name
is sftp, matched whole. DoChannelRequest() keeps the parsed length in
channel->commandSz, so neither wolfSSH_SFTP_accept()'s grant gate nor
wolfSSH_accept()'s divert serves "sftpx" or "sftp\0evil".

- cover a granted name longer than sftp, one of its length, and one
  running past an embedded NUL
- cover the divert with those three names and a control that diverts
- exec keeps its command length too

Issue: F-11665
pull/1235/head
John Safranek 2026-09-09 11:50:29 -07:00 committed by Paul Adelsbach
parent 7adf524f6f
commit 3516fb00a0
5 changed files with 174 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/internal.c
View File

@ -13158,7 +13158,8 @@ static int DoChannelRequest(WOLFSSH* ssh,
ssh->clientState = CLIENT_DONE;
}
else if (ChannelRequestIs(type, typeSz, "exec")) {
ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
ret = GetStringAlloc(ssh->ctx->heap,
&channel->command, &channel->commandSz,
buf, len, &begin);
if (ret == WS_SUCCESS)
WLOG(WS_LOG_DEBUG, " command = %s", channel->command);
@ -13175,7 +13176,8 @@ static int DoChannelRequest(WOLFSSH* ssh,
ssh->clientState = CLIENT_DONE;
}
else if (ChannelRequestIs(type, typeSz, "subsystem")) {
ret = GetStringAlloc(ssh->ctx->heap, &channel->command, NULL,
ret = GetStringAlloc(ssh->ctx->heap,
&channel->command, &channel->commandSz,
buf, len, &begin);
if (ret == WS_SUCCESS)
WLOG(WS_LOG_DEBUG, " subsystem = %s", channel->command);

4
src/ssh.c
View File

@ -822,7 +822,9 @@ int wolfSSH_accept(WOLFSSH* ssh)
const char* cmd = wolfSSH_GetSessionCommand(ssh);
if (cmd != NULL &&
WOLFSSH_SESSION_SUBSYSTEM == wolfSSH_GetSessionType(ssh)
&& (WSTRNCMP(cmd, "sftp", 4) == 0)) {
&& ssh->channelList->commandSz ==
(word32)WSTRLEN("sftp")
&& (WSTRCMP(cmd, "sftp") == 0)) {
ssh->acceptState = ACCEPT_INIT_SFTP;
return wolfSSH_SFTP_accept(ssh);
}

7
src/wolfsftp.c
View File

@ -1394,13 +1394,16 @@ int wolfSSH_SFTP_accept(WOLFSSH* ssh)
* callback: serve only a session channel it granted sftp on. The
* request having named sftp is not enough, so this asks for the
* grant as well -- unlike wolfSSH_accept()'s divert, which reads
* only the type and command. */
* only the type and command. The name matches whole, length
* and bytes: sftpx, or sftp with an embedded NUL, is some
* other subsystem. */
const WOLFSSH_CHANNEL* channel = ssh->channelList;
if (channel == NULL || !channel->sessionGranted
|| channel->sessionType != WOLFSSH_SESSION_SUBSYSTEM
|| channel->command == NULL
|| WSTRNCMP(channel->command, "sftp", 4) != 0) {
|| channel->commandSz != (word32)WSTRLEN("sftp")
|| WSTRCMP(channel->command, "sftp") != 0) {
WLOG(WS_LOG_SFTP, "No sftp subsystem granted on the session");
return WS_INVALID_STATE_E;
}

161
tests/regress.c
View File

@ -4108,6 +4108,100 @@ static void TestSftpAcceptAppChannelsRefusesShell(void)
FreeChannelOpenHarness(&harness);
}
/* A name that only starts with sftp is not an sftp grant. */
static void TestSftpAcceptAppChannelsRefusesPrefixName(void)
{
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
byte in[64];
word32 inSz;
channel = SeedAppChannelsSession(&harness, "subsystem", "sftpx");
inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in));
RepointHarnessInput(&harness, in, inSz);
AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E);
AssertIntEQ(harness.io.outSz, 0);
AssertIntEQ(harness.io.inOff, 0);
AssertIntEQ(harness.ssh->error, WS_SUCCESS);
FreeChannelOpenHarness(&harness);
}
/* A granted name whose wire length runs past an embedded NUL is not an
* sftp grant: the four bytes ahead of the NUL match, the name does not. */
static void TestSftpAcceptAppChannelsRefusesNulName(void)
{
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
static const byte nulName[] = {
's', 'f', 't', 'p', 0, 'e', 'v', 'i', 'l'
};
byte payload[128];
byte in[128];
word32 idx = 0;
word32 inSz;
sessionReqCbCalls = 0;
sessionReqCbReturn = 0;
InitChannelOpenHarness(&harness, NULL, 0);
AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS);
AssertIntEQ(wolfSSH_CTX_SetChannelReqSubsysCb(harness.ctx,
RecordingSessionReqCb), 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 name as a C string, which cannot carry the NUL. */
idx = AppendUint32(payload, sizeof(payload), idx, channel->channel);
idx = AppendString(payload, sizeof(payload), idx, "subsystem");
idx = AppendByte(payload, sizeof(payload), idx, 1);
idx = AppendUint32(payload, sizeof(payload), idx, (word32)sizeof(nulName));
idx = AppendData(payload, sizeof(payload), idx, nulName, sizeof(nulName));
inSz = WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, in, sizeof(in));
RepointHarnessInput(&harness, in, inSz);
/* The callback reads a C string, so it sees sftp and grants it. */
AssertIntEQ(DoReceive(harness.ssh), WS_SUCCESS);
AssertIntEQ(sessionReqCbCalls, 1);
AssertIntEQ(WSTRCMP(sessionReqCbCommand, "sftp"), 0);
AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz),
MSGID_CHANNEL_SUCCESS);
inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in));
RepointHarnessInput(&harness, in, inSz);
AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E);
AssertIntEQ(harness.io.outSz, 0);
AssertIntEQ(harness.io.inOff, 0);
AssertIntEQ(harness.ssh->error, WS_SUCCESS);
FreeChannelOpenHarness(&harness);
}
/* Four bytes that are not sftp are not an sftp grant. */
static void TestSftpAcceptAppChannelsRefusesSameLengthName(void)
{
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
byte in[64];
word32 inSz;
channel = SeedAppChannelsSession(&harness, "subsystem", "sfxp");
inSz = BuildSftpInitDataPacket(channel->channel, in, sizeof(in));
RepointHarnessInput(&harness, in, inSz);
AssertIntEQ(wolfSSH_SFTP_accept(harness.ssh), WS_INVALID_STATE_E);
AssertIntEQ(harness.io.outSz, 0);
AssertIntEQ(harness.io.inOff, 0);
AssertIntEQ(harness.ssh->error, WS_SUCCESS);
FreeChannelOpenHarness(&harness);
}
/* The grant the mode relies on: the subsystem callback took sftp, so the
* INIT is answered with a VERSION and accept() stays parked. */
static void TestSftpAcceptAppChannelsServesGrantedSftp(void)
@ -4237,6 +4331,69 @@ static void TestSftpAcceptAppChannelsRefusesRejectedCb(void)
}
/* wolfSSH_accept()'s divert to the built-in server matches the subsystem
* name whole, by length as well as bytes. The last case is the control:
* with no name that does divert, a harness that never reached the check
* would pass every refusal above it. */
static void TestAcceptDivertMatchesSftpNameWhole(void)
{
static const struct {
const char* name;
word32 nameSz;
byte divert;
} cases[] = {
{ "sftpx", 5, 0 }, /* longer than sftp */
{ "sfxp", 4, 0 }, /* the length of sftp, other bytes */
{ "sftp\0evil", 9, 0 }, /* sftp up to an embedded NUL */
{ "sftp", 4, 1 },
};
ChannelOpenHarness harness;
WOLFSSH_CHANNEL* channel;
byte payload[128];
byte in[128];
word32 idx;
word32 inSz;
word32 i;
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, "subsystem");
idx = AppendByte(payload, sizeof(payload), idx, 1);
idx = AppendUint32(payload, sizeof(payload), idx, cases[i].nameSz);
idx = AppendData(payload, sizeof(payload), idx,
(const byte*)cases[i].name, cases[i].nameSz);
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].nameSz);
RepointHarnessInput(&harness, NULL, 0);
harness.ssh->acceptState = ACCEPT_SERVER_CHANNEL_ACCEPT_SENT;
if (cases[i].divert) {
/* The built-in server has the session, and stops on the INIT
* the empty input cannot supply. */
wolfSSH_accept(harness.ssh);
AssertIntEQ(harness.ssh->acceptState, ACCEPT_INIT_SFTP);
}
else {
AssertIntEQ(wolfSSH_accept(harness.ssh), WS_SUCCESS);
AssertIntEQ(harness.ssh->acceptState,
ACCEPT_CLIENT_SESSION_ESTABLISHED);
}
FreeChannelOpenHarness(&harness);
}
}
#endif /* WOLFSSH_SFTP */
/* A username change after the first userauth request must end the session. */
@ -14308,11 +14465,15 @@ int main(int argc, char** argv)
TestSftpAcceptAppChannelsNeedsSession();
TestSftpAcceptAppChannelsRefusesPreAccept();
TestSftpAcceptAppChannelsRefusesShell();
TestSftpAcceptAppChannelsRefusesPrefixName();
TestSftpAcceptAppChannelsRefusesNulName();
TestSftpAcceptAppChannelsRefusesSameLengthName();
TestSftpAcceptAppChannelsServesGrantedSftp();
TestSftpAcceptAppChannelsRefusesEstablishedShell();
TestSftpAcceptAppChannelsServesEstablishedSftp();
TestSftpAcceptAppChannelsRefusesNoCb();
TestSftpAcceptAppChannelsRefusesRejectedCb();
TestAcceptDivertMatchesSftpNameWhole();
#endif
TestSecondSessionChannelRejected();
TestUsernameChangeDisconnects();

1
wolfssh/internal.h
View File

@ -1452,6 +1452,7 @@ struct WOLFSSH_CHANNEL {
* Accumulates unread data, does not overwrite
* it. */
char* command;
word32 commandSz;
struct WOLFSSH* ssh;
struct WOLFSSH_CHANNEL* next;
};