Skip to content

Add a safe mask for peer-supplied mode and extend wolfSSH_SFTP_Open #999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
+275 −57
Open

Add a safe mask for peer-supplied mode and extend wolfSSH_SFTP_Open #999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 164 additions & 44 deletions examples/sftpclient/sftpclient.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ static void ShowCommands(void)
printf("\n\nCommands :\n");
printf("\tcd <string> change directory\n");
printf("\tchmod <mode> <path> change mode\n");
printf("\tcreat <mode> <path> create file with given permissions\n");
printf("\tget <remote file> <local file> pulls file(s) from server\n");
printf("\tlcd <path> change local directory\n");
printf("\tlls list local directory\n");
Expand Down Expand Up @@ -433,6 +434,76 @@ static INLINE char* SFTP_FGETS(func_args* args, char* msg, int msgSz)
}


/*
* Parse "<mode> <path>" from a SFTP command argument string.
* pt: points past the command keyword (caller advanced by sizeof).
* modeBuf: caller buffer of WOLFSSH_MAX_OCTET_LEN bytes; receives the
* null-terminated octal mode token on success.
* pathOut: receives a pointer to the resolved remote path on success.
* allocOut: receives a malloc'd absolute-path buffer when pt was relative
* (caller must WFREE); NULL when the path was already absolute.
* remoteDir: current remote working directory.
* Returns 0 on success,
* 1 if no mode token found (caller prints error and continues),
* -1 on malloc failure (caller returns -1).
*/
static int sftpParseModeAndPath(char* pt, char* modeBuf, char** pathOut,
char** allocOut, const char* remoteDir)
{
word32 sz, idx;
char* f;

*allocOut = NULL;
*pathOut = NULL;

sz = (word32)WSTRLEN(pt);
if (sz > 0 && pt[sz - 1] == '\n') {
pt[sz - 1] = '\0';
sz--;
}

for (idx = 0; idx < sz && pt[0] == ' '; idx++, pt++);
sz = (word32)WSTRLEN(pt);

sz = (sz < WOLFSSH_MAX_OCTET_LEN - 1) ? sz : WOLFSSH_MAX_OCTET_LEN - 1;
WMEMCPY(modeBuf, pt, sz);
modeBuf[sz] = '\0';
for (idx = 0; idx < sz; idx++) {
if (modeBuf[idx] == ' ') {
modeBuf[idx] = '\0';
break;
}
}
if (idx == 0 || (idx == sz && pt[sz] != ' '))
return 1;
Comment thread
yosuke-wolfssl marked this conversation as resolved.

pt += (word32)WSTRLEN(modeBuf);
sz = (word32)WSTRLEN(pt);
for (idx = 0; idx < sz && pt[0] == ' '; idx++, pt++);

if (pt[0] == '\0')
return 1;

if (pt[0] != '/') {
int maxSz = (int)WSTRLEN(remoteDir) + (int)WSTRLEN(pt) + 2;
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (f == NULL)
return -1;
f[0] = '\0';
WSTRNCAT(f, remoteDir, maxSz);
if (WSTRLEN(remoteDir) > 1)
WSTRNCAT(f, "/", maxSz);
WSTRNCAT(f, pt, maxSz);
*allocOut = f;
*pathOut = f;
}
else {
*pathOut = pt;
}
return 0;
}


/* main loop for handling commands */
static int doCmds(func_args* args)
{
Expand Down Expand Up @@ -822,54 +893,20 @@ static int doCmds(func_args* args)
}

if ((pt = WSTRNSTR(msg, "chmod", MAX_CMD_SZ)) != NULL) {
word32 sz, idx;
char* f = NULL;
char mode[WOLFSSH_MAX_OCTET_LEN];
char* path;
char mode[WOLFSSH_MAX_OCTET_LEN];
int parseRet;

pt += sizeof("chmod");
sz = (word32)WSTRLEN(pt);

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

/* advance pointer to first location of non space character */
for (idx = 0; idx < sz && pt[0] == ' '; idx++, pt++);
sz = (word32)WSTRLEN(pt);

/* get mode */
sz = (sz < WOLFSSH_MAX_OCTET_LEN - 1)? sz :
WOLFSSH_MAX_OCTET_LEN -1;
WMEMCPY(mode, pt, sz);
mode[WOLFSSH_MAX_OCTET_LEN - 1] = '\0';
for (idx = 0; idx < sz; idx++) {
if (mode[idx] == ' ') {
mode[idx] = '\0';
break;
}
}
if (idx == 0) {
parseRet = sftpParseModeAndPath(pt, mode, &path, &f, workingDir);
if (parseRet == 1) {
printf("error with getting mode\r\n");
continue;
}
pt += (word32)WSTRLEN(mode);
sz = (word32)WSTRLEN(pt);
for (idx = 0; idx < sz && pt[0] == ' '; idx++, pt++);

if (pt[0] != '/') {
int maxSz = (int)WSTRLEN(workingDir) + sz + 2;
f = (char*)WMALLOC(maxSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (f == NULL) {
err_msg("Error malloc'ing");
return -1;
}

f[0] = '\0';
WSTRNCAT(f, workingDir, maxSz);
if (WSTRLEN(workingDir) > 1) {
WSTRNCAT(f, "/", maxSz);
}
WSTRNCAT(f, pt, maxSz);

pt = f;
if (parseRet == -1) {
err_msg("Error malloc'ing");
return -1;
}

/* update permissions */
Expand All @@ -881,21 +918,104 @@ static int doCmds(func_args* args)
}
}

ret = wolfSSH_SFTP_CHMOD(ssh, pt, mode);
ret = wolfSSH_SFTP_CHMOD(ssh, path, mode);
err = wolfSSH_get_error(ssh);
} while ((err == WS_WANT_READ || err == WS_WANT_WRITE ||
err == WS_REKEYING) && ret != WS_SUCCESS);
if (ret != WS_SUCCESS) {
if (SFTP_FPUTS(args, "Unable to change permissions of ") < 0) {
err_msg("fputs error");
WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return -1;
}
if (SFTP_FPUTS(args, pt) < 0) {
if (SFTP_FPUTS(args, path) < 0) {
err_msg("fputs error");
WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return -1;
}
if (SFTP_FPUTS(args, "\n") < 0) {
err_msg("fputs error");
WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return -1;
}
}

WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
continue;
}

if ((pt = WSTRNSTR(msg, "creat", MAX_CMD_SZ)) != NULL) {
char* f = NULL;
char* path;
char mode[WOLFSSH_MAX_OCTET_LEN];
byte handle[WOLFSSH_MAX_HANDLE];
word32 handleSz = WOLFSSH_MAX_HANDLE;
WS_SFTP_FILEATRB atr;
int parseRet;
int openRet = WS_FATAL_ERROR;
unsigned long perVal;
char* modeEnd;

pt += sizeof("creat");
parseRet = sftpParseModeAndPath(pt, mode, &path, &f, workingDir);
if (parseRet == 1) {
printf("error with getting mode\r\n");
continue;
}
if (parseRet == -1) {
err_msg("Error malloc'ing");
return -1;
}

/* build permission attribute from octal mode string;
* wolfSSH_oct2dec is internal scope so strtoul is used here */
perVal = strtoul(mode, &modeEnd, 8);
if (*modeEnd == '\0' && perVal <= 07777) {
WMEMSET(&atr, 0, sizeof(WS_SFTP_FILEATRB));
atr.flags = WOLFSSH_FILEATRB_PERM;
atr.per = (word32)perVal;

/* open (create) remote file with the given permissions */
handleSz = WOLFSSH_MAX_HANDLE;
do {
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
ret = wolfSSH_worker(ssh, NULL);
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
ret = wolfSSH_get_error(ssh);
}
}
ret = wolfSSH_SFTP_Open(ssh, path,
WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT |
WOLFSSH_FXF_TRUNC, &atr, handle, &handleSz);
err = wolfSSH_get_error(ssh);
} while ((err == WS_WANT_READ || err == WS_WANT_WRITE ||
err == WS_REKEYING) && ret != WS_SUCCESS);
openRet = ret;
}
if (openRet == WS_SUCCESS) {
do {
while (ret == WS_REKEYING || ssh->error == WS_REKEYING) {
ret = wolfSSH_worker(ssh, NULL);
if (ret != WS_SUCCESS && ret == WS_FATAL_ERROR) {
ret = wolfSSH_get_error(ssh);
}
}
ret = wolfSSH_SFTP_Close(ssh, handle, handleSz);
err = wolfSSH_get_error(ssh);
} while ((err == WS_WANT_READ || err == WS_WANT_WRITE ||
err == WS_REKEYING) && ret != WS_SUCCESS);
if (ret != WS_SUCCESS) {
if (SFTP_FPUTS(args, "Unable to close file handle\n") < 0) {
err_msg("fputs error");
WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return -1;
}
}
}
else {
if (SFTP_FPUTS(args, "Unable to create file\n") < 0) {
err_msg("fputs error");
WFREE(f, NULL, DYNAMIC_TYPE_TMP_BUFFER);
return -1;
}
}
Expand Down
37 changes: 26 additions & 11 deletions src/wolfsftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1872,9 +1872,9 @@ int wolfSSH_SFTP_RecvMKDIR(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
#ifndef USE_WINDOWS_API
#ifndef WOLFSSH_FATFS
{
word32 mode = 040755;
word32 mode = 0755;
if (atr.flags & WOLFSSH_FILEATRB_PERM) {
mode = atr.per;
mode = WOLFSSH_SFTP_SAFE_MODE(atr.per);
}
else {
WLOG(WS_LOG_SFTP, "No permission attribute, using default");
Expand Down Expand Up @@ -2144,7 +2144,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
}
}
#else
fd = WOPEN(ssh->fs, dir, m, atr.per);
fd = WOPEN(ssh->fs, dir, m, WOLFSSH_SFTP_SAFE_MODE(atr.per));
#endif
if (fd < 0) {
WLOG(WS_LOG_SFTP, "Error opening file %s", dir);
Expand Down Expand Up @@ -5581,7 +5581,7 @@ static int SFTP_SetFileAttributes(WOLFSSH* ssh,
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR)
/* check if permissions attribute present */
if (atr->flags & WOLFSSH_FILEATRB_PERM) {
ret = SFTP_SetMode(ssh->fs, name, atr->per);
ret = SFTP_SetMode(ssh->fs, name, WOLFSSH_SFTP_SAFE_MODE(atr->per));
}
#endif

Expand Down Expand Up @@ -5622,7 +5622,7 @@ static int SFTP_SetFileAttributesHandle(WOLFSSH* ssh,
#ifndef USE_WINDOWS_API
/* check if permissions attribute present */
if (atr->flags & WOLFSSH_FILEATRB_PERM) {
ret = SFTP_SetModeHandle(ssh->fs, handle, atr->per);
ret = SFTP_SetModeHandle(ssh->fs, handle, WOLFSSH_SFTP_SAFE_MODE(atr->per));
}
#endif

Expand Down Expand Up @@ -7280,7 +7280,8 @@ int wolfSSH_SFTP_SetSTAT(WOLFSSH* ssh, char* dir, WS_SFTP_FILEATRB* atr)
* dir NULL terminated string holding file name
* reason the reason file is being opened for i.e. WOLFSSH_FXF_READ,
* WOLFSSH_FXF_WRITE, ....
* atr attributes of file
* atr attributes serialized into the SSH_FXP_OPEN packet; NULL sends an
* empty attributes block (flags=0), preserving the prior behavior
* handle resulting handle from opening the file
* handleSz gets set to resulting handle size. Should initially be size of
* handle buffer when passed in
Expand All @@ -7293,6 +7294,7 @@ int wolfSSH_SFTP_Open(WOLFSSH* ssh, char* dir, word32 reason,
WS_SFTP_OPEN_STATE* state = NULL;
int ret = WS_SUCCESS;
int sz;
int atrSz;

WLOG(WS_LOG_SFTP, "Entering wolfSSH_SFTP_Open()");
if (ssh == NULL || dir == NULL) {
Expand Down Expand Up @@ -7320,8 +7322,10 @@ int wolfSSH_SFTP_Open(WOLFSSH* ssh, char* dir, word32 reason,
case STATE_OPEN_INIT:
WLOG(WS_LOG_SFTP, "SFTP OPEN STATE: INIT");
sz = (int)WSTRLEN(dir);
atrSz = (atr != NULL) ? SFTP_AttributesSz(ssh, atr) :
UINT32_SZ;
if (wolfSSH_SFTP_buffer_create(ssh, &state->buffer, sz +
WOLFSSH_SFTP_HEADER + UINT32_SZ * 3) !=
WOLFSSH_SFTP_HEADER + UINT32_SZ * 2 + atrSz) !=
WS_SUCCESS) {
ssh->error = WS_MEMORY_E;
ret = WS_FATAL_ERROR;
Expand All @@ -7330,7 +7334,7 @@ int wolfSSH_SFTP_Open(WOLFSSH* ssh, char* dir, word32 reason,
}

ret = SFTP_SetHeader(ssh, ssh->reqId, WOLFSSH_FTP_OPEN,
sz + UINT32_SZ * 3,
sz + UINT32_SZ * 2 + atrSz,
wolfSSH_SFTP_buffer_data(&state->buffer));
if (ret != WS_SUCCESS) {
state->state = STATE_OPEN_CLEANUP;
Expand All @@ -7347,9 +7351,20 @@ int wolfSSH_SFTP_Open(WOLFSSH* ssh, char* dir, word32 reason,
wolfSSH_SFTP_buffer_idx(&state->buffer), sz);
wolfSSH_SFTP_buffer_c32toa(&state->buffer, reason);

/* @TODO handle adding attributes here */
WOLFSSH_UNUSED(atr);
wolfSSH_SFTP_buffer_c32toa(&state->buffer, 0x00000000);
if (atr != NULL) {
ret = SFTP_SetAttributes(ssh,
Comment thread
yosuke-wolfssl marked this conversation as resolved.
Comment thread
yosuke-wolfssl marked this conversation as resolved.
wolfSSH_SFTP_buffer_data(&state->buffer) +
wolfSSH_SFTP_buffer_idx(&state->buffer), atrSz, atr);
if (ret != WS_SUCCESS) {
state->state = STATE_OPEN_CLEANUP;
continue;
}
wolfSSH_SFTP_buffer_seek(&state->buffer,
wolfSSH_SFTP_buffer_idx(&state->buffer), atrSz);
}
else {
wolfSSH_SFTP_buffer_c32toa(&state->buffer, 0x00000000);
}
ret = wolfSSH_SFTP_buffer_set_size(&state->buffer,
wolfSSH_SFTP_buffer_idx(&state->buffer));
if (ret != WS_SUCCESS) {
Expand Down
Loading
Loading