Skip to content

Commit 5272cc4

Browse files
ejohnstownejohnstown
authored
Merge pull request wolfSSL#911 from padelsbach/sftp-command-underflow
Sftp command underflow
2 parents 4ed01d3 + 94b8271 commit 5272cc4

File tree

2 files changed

+152
-99
lines changed

2 files changed

+152
-99
lines changed

‎examples/sftpclient/sftpclient.c‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ static int doCmds(func_args* args)
472472
pt += sizeof("mkdir");
473473
sz = (int)WSTRLEN(pt);
474474

475-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
475+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
476476
if (pt[0] != '/') {
477477
int maxSz = (int)WSTRLEN(workingDir) + sz + 2;
478478
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -522,7 +522,7 @@ static int doCmds(func_args* args)
522522
pt += sizeof("get");
523523

524524
sz = (int)WSTRLEN(pt);
525-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
525+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
526526

527527
/* search for space delimiter */
528528
to = pt;
@@ -633,7 +633,7 @@ static int doCmds(func_args* args)
633633
pt += sizeof("put");
634634
sz = (int)WSTRLEN(pt);
635635

636-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
636+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
637637

638638
to = pt;
639639
for (i = 0; i < sz; i++) {
@@ -739,7 +739,7 @@ static int doCmds(func_args* args)
739739
pt += sizeof("cd");
740740
sz = (int)WSTRLEN(pt);
741741

742-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
742+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
743743
if (pt[0] != '/') {
744744
int maxSz = (int)WSTRLEN(workingDir) + sz + 2;
745745
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -803,7 +803,7 @@ static int doCmds(func_args* args)
803803
pt += sizeof("chmod");
804804
sz = (word32)WSTRLEN(pt);
805805

806-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
806+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
807807

808808
/* advance pointer to first location of non space character */
809809
for (idx = 0; idx < sz && pt[0] == ' '; idx++, pt++);
@@ -885,7 +885,7 @@ static int doCmds(func_args* args)
885885
pt += sizeof("rmdir");
886886
sz = (int)WSTRLEN(pt);
887887

888-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
888+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
889889
if (pt[0] != '/') {
890890
int maxSz = (int)WSTRLEN(workingDir) + sz + 2;
891891
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -942,7 +942,7 @@ static int doCmds(func_args* args)
942942
pt += sizeof("rm");
943943
sz = (int)WSTRLEN(pt);
944944

945-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
945+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
946946
if (pt[0] != '/') {
947947
int maxSz = (int)WSTRLEN(workingDir) + sz + 2;
948948
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -997,7 +997,7 @@ static int doCmds(func_args* args)
997997
pt += sizeof("rename");
998998
sz = (int)WSTRLEN(pt);
999999

1000-
if (pt[sz - 1] == '\n') pt[sz - 1] = '\0';
1000+
if (sz > 0 && pt[sz - 1] == '\n') pt[sz - 1] = '\0';
10011001

10021002
/* search for space delimiter */
10031003
to = pt;

‎tests/sftp.c‎

Lines changed: 144 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -43,108 +43,153 @@
4343
#include "examples/echoserver/echoserver.h"
4444
#include "examples/sftpclient/sftpclient.h"
4545

46-
static const char* cmds[] = {
47-
"mkdir a",
48-
"cd a",
49-
"pwd",
50-
"ls",
51-
#ifdef WOLFSSH_ZEPHYR
52-
"put " CONFIG_WOLFSSH_SFTP_DEFAULT_DIR "/configure.ac",
53-
#else
54-
"put configure.ac",
55-
#endif
56-
"ls",
57-
#ifdef WOLFSSH_ZEPHYR
58-
"get configure.ac " CONFIG_WOLFSSH_SFTP_DEFAULT_DIR "/test-get",
59-
#else
60-
"get configure.ac test-get",
61-
#endif
62-
"rm configure.ac",
63-
"cd ../",
64-
"ls",
65-
"rename test-get test-get-2",
66-
"rmdir a",
67-
"ls",
68-
"chmod 600 test-get-2",
69-
"rm test-get-2",
70-
"ls -s",
71-
"exit"
72-
};
73-
static int commandIdx = 0;
46+
/*
47+
* Each test command is paired with an optional check function that
48+
* validates the output it produces. This eliminates the fragile
49+
* index-based coupling between the command array and the validator.
50+
*/
51+
typedef int (*SftpTestCheck)(void);
52+
53+
typedef struct {
54+
const char* cmd;
55+
SftpTestCheck check; /* validates output from THIS cmd, or NULL */
56+
} SftpTestCmd;
57+
58+
/* Test buffer */
7459
static char inBuf[1024] = {0};
7560

76-
/* return 0 on success */
77-
static int Expected(int command)
61+
/* check that pwd output ends in /a */
62+
static int checkPwdInA(void)
7863
{
7964
int i;
65+
int len;
8066

81-
switch (command) {
82-
case 3: /* pwd */
83-
/* working directory should not contain a '.' at the end */
84-
for (i = 0; i < (int)sizeof(inBuf); i++) {
85-
if (inBuf[i] == '\n') {
86-
inBuf[i] = '\0';
87-
break;
88-
}
89-
}
90-
91-
if (inBuf[WSTRLEN(inBuf) - 2] != '/') {
92-
printf("unexpected pwd of %s\n looking for '/'\n", inBuf);
93-
return -1;
94-
}
95-
if (inBuf[WSTRLEN(inBuf) - 1] != 'a') {
96-
printf("unexpected pwd of %s\n looking for 'a'\n", inBuf);
97-
return -1;
98-
}
67+
for (i = 0; i < (int)sizeof(inBuf); i++) {
68+
if (inBuf[i] == '\n') {
69+
inBuf[i] = '\0';
9970
break;
71+
}
72+
}
10073

101-
case 4:
102-
{
74+
len = (int)WSTRLEN(inBuf);
75+
if (len < 2) {
76+
printf("pwd output too short: %s\n", inBuf);
77+
return -1;
78+
}
79+
if (inBuf[len - 2] != '/') {
80+
printf("unexpected pwd of %s, looking for '/'\n", inBuf);
81+
return -1;
82+
}
83+
if (inBuf[len - 1] != 'a') {
84+
printf("unexpected pwd of %s, looking for 'a'\n", inBuf);
85+
return -1;
86+
}
87+
return 0;
88+
}
89+
90+
/* check that ls of empty dir shows only . and .. */
91+
static int checkLsEmpty(void)
92+
{
10393
#ifdef WOLFSSH_ZEPHYR
104-
/* No . and .. in zephyr fs API */
105-
char expt1[] = "wolfSSH sftp> ";
106-
char expt2[] = "wolfSSH sftp> ";
94+
/* No . and .. in zephyr fs API */
95+
char expt1[] = "wolfSSH sftp> ";
96+
char expt2[] = "wolfSSH sftp> ";
10797
#else
108-
char expt1[] = ".\n..\nwolfSSH sftp> ";
109-
char expt2[] = "..\n.\nwolfSSH sftp> ";
98+
char expt1[] = ".\n..\nwolfSSH sftp> ";
99+
char expt2[] = "..\n.\nwolfSSH sftp> ";
110100
#endif
111-
if (WMEMCMP(expt1, inBuf, sizeof(expt1)) != 0 &&
112-
WMEMCMP(expt2, inBuf, sizeof(expt2)) != 0) {
113-
printf("unexpected ls\n");
114-
printf("\texpected \n%s\n\tor\n%s\n\tbut got\n%s\n", expt1,
115-
expt2, inBuf);
116-
return -1;
117-
}
118-
else
119-
return 0;
120-
121-
}
122-
123-
case 6:
124-
if (WSTRNSTR(inBuf, "configure.ac", sizeof(inBuf)) == NULL) {
125-
fprintf(stderr, "configure.ac not found in %s\n", inBuf);
126-
return 1;
127-
}
128-
else {
129-
return 0;
130-
}
101+
if (WMEMCMP(expt1, inBuf, sizeof(expt1)) != 0 &&
102+
WMEMCMP(expt2, inBuf, sizeof(expt2)) != 0) {
103+
printf("unexpected ls\n");
104+
printf("\texpected \n%s\n\tor\n%s\n\tbut got\n%s\n",
105+
expt1, expt2, inBuf);
106+
return -1;
107+
}
108+
return 0;
109+
}
131110

132-
case 10:
133-
return (WSTRNSTR(inBuf, "test-get", sizeof(inBuf)) == NULL);
111+
/* check that ls output contains a specific file */
112+
static int checkLsHasConfigureAc(void)
113+
{
114+
if (WSTRNSTR(inBuf, "configure.ac", sizeof(inBuf)) == NULL) {
115+
fprintf(stderr, "configure.ac not found in %s\n", inBuf);
116+
return 1;
117+
}
118+
return 0;
119+
}
134120

135-
case 13:
136-
return (WSTRNSTR(inBuf, "test-get-2", sizeof(inBuf)) == NULL);
121+
static int checkLsHasTestGet(void)
122+
{
123+
return (WSTRNSTR(inBuf, "test-get",
124+
sizeof(inBuf)) == NULL) ? 1 : 0;
125+
}
137126

138-
case 16:
139-
return (WSTRNSTR(inBuf, "size in bytes", sizeof(inBuf)) == NULL);
127+
static int checkLsHasTestGet2(void)
128+
{
129+
return (WSTRNSTR(inBuf, "test-get-2",
130+
sizeof(inBuf)) == NULL) ? 1 : 0;
131+
}
140132

141-
default:
142-
break;
143-
}
144-
WMEMSET(inBuf, 0, sizeof(inBuf));
145-
return 0;
133+
static int checkLsSize(void)
134+
{
135+
return (WSTRNSTR(inBuf, "size in bytes",
136+
sizeof(inBuf)) == NULL) ? 1 : 0;
146137
}
147138

139+
static const SftpTestCmd cmds[] = {
140+
/* If a prior run was interrupted, files and directories
141+
* created during the test may still exist in the working
142+
* directory, causing mkdir to fail and ls checks to see
143+
* unexpected entries. Remove them here before starting.
144+
* These run as SFTP commands rather than local syscalls
145+
* so they are portable across all platforms (Windows,
146+
* Zephyr, POSIX). Failures are silently ignored since
147+
* the files may not exist. */
148+
{ "rm a/configure.ac", NULL },
149+
{ "rmdir a", NULL },
150+
{ "rm test-get", NULL },
151+
{ "rm test-get-2", NULL },
152+
153+
/* --- test sequence starts here --- */
154+
{ "mkdir a", NULL },
155+
{ "cd a", NULL },
156+
{ "pwd", checkPwdInA },
157+
{ "ls", checkLsEmpty },
158+
#ifdef WOLFSSH_ZEPHYR
159+
{ "put " CONFIG_WOLFSSH_SFTP_DEFAULT_DIR "/configure.ac", NULL },
160+
#else
161+
{ "put configure.ac", NULL },
162+
#endif
163+
{ "ls", checkLsHasConfigureAc },
164+
#ifdef WOLFSSH_ZEPHYR
165+
{ "get configure.ac "
166+
CONFIG_WOLFSSH_SFTP_DEFAULT_DIR "/test-get", NULL },
167+
#else
168+
{ "get configure.ac test-get", NULL },
169+
#endif
170+
{ "rm configure.ac", NULL },
171+
{ "cd ../", NULL },
172+
{ "ls", checkLsHasTestGet },
173+
{ "rename test-get test-get-2", NULL },
174+
{ "rmdir a", NULL },
175+
{ "ls", checkLsHasTestGet2 },
176+
{ "chmod 600 test-get-2", NULL },
177+
{ "rm test-get-2", NULL },
178+
{ "ls -s", checkLsSize },
179+
/* empty arg tests: must not underflow on pt[sz-1] */
180+
{ "mkdir", NULL },
181+
{ "cd", NULL },
182+
{ "ls", NULL },
183+
{ "chmod", NULL },
184+
{ "rmdir", NULL },
185+
{ "rm", NULL },
186+
{ "rename", NULL },
187+
{ "get", NULL },
188+
{ "put", NULL },
189+
{ "exit", NULL },
190+
};
191+
static int commandIdx = 0;
192+
148193

149194
static int commandCb(const char* in, char* out, int outSz)
150195
{
@@ -157,18 +202,26 @@ static int commandCb(const char* in, char* out, int outSz)
157202

158203
/* get command input */
159204
if (out) {
160-
int sz = (int)WSTRLEN(cmds[commandIdx]);
205+
int sz = (int)WSTRLEN(cmds[commandIdx].cmd);
161206
if (outSz < sz) {
162207
ret = -1;
163208
}
164209
else {
165-
WMEMCPY(out, cmds[commandIdx], sz);
210+
WMEMCPY(out, cmds[commandIdx].cmd, sz);
166211
}
167212

168-
if (Expected(commandIdx) != 0) {
169-
fprintf(stderr, "Failed on command index %d\n", commandIdx);
170-
exit(1); /* abort out */
213+
/* validate output from the previous command */
214+
if (commandIdx > 0 &&
215+
cmds[commandIdx - 1].check != NULL) {
216+
if (cmds[commandIdx - 1].check() != 0) {
217+
fprintf(stderr,
218+
"Check failed for \"%s\" (index %d)\n",
219+
cmds[commandIdx - 1].cmd,
220+
commandIdx - 1);
221+
exit(1);
222+
}
171223
}
224+
WMEMSET(inBuf, 0, sizeof(inBuf));
172225
commandIdx++;
173226
}
174227
return ret;

0 commit comments

Comments
 (0)