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

pull/1000/head
Yosuke Shimizu 2026-06-02 16:19:11 +09:00 committed by John Safranek
parent 4ebb6be336
commit ca803ec2cc
4 changed files with 275 additions and 57 deletions

View File

@ -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");
@ -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;
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)
{
@ -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 */
@ -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;
}
}

View File

@ -1926,9 +1926,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");
@ -2269,7 +2269,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);
@ -5717,7 +5717,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
@ -5758,7 +5758,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
@ -7423,7 +7423,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
@ -7436,6 +7437,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) {
@ -7463,8 +7465,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;
@ -7473,7 +7477,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;
@ -7490,9 +7494,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,
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) {

View File

@ -147,6 +147,68 @@ static int checkCdNonexistent(void)
return 0;
}
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_FATFS) && \
!defined(WOLFSSH_ZEPHYR)
/* Captured once before any threads start; used to compute the expected
* post-open file mode without changing process-wide umask state. */
static mode_t sftpTestUmask = 0;
/* Verify SFTP_SetFileAttributes stripped setuid/setgid/sticky bits when
* the client sent chmod 4777 (setuid + rwxrwxrwx). */
static int checkChmodStripsSpecialBits(void)
{
WSTAT_T st;
WMEMSET(&st, 0, sizeof(WSTAT_T));
if (WSTAT(NULL, "test-get-2", &st) != 0) {
fprintf(stderr, "stat test-get-2 failed\n");
return 1;
}
if (st.st_mode & 07000) {
fprintf(stderr,
"WOLFSSH_SFTP_SAFE_MODE: special bits not stripped: mode=%06o\n",
(unsigned)(st.st_mode & 07777));
return 1;
}
if ((st.st_mode & 0777) != 0777) {
fprintf(stderr,
"WOLFSSH_SFTP_SAFE_MODE: unexpected base mode=%06o, want 0777\n",
(unsigned)(st.st_mode & 07777));
return 1;
}
return 0;
}
/* Verify wolfSSH_SFTP_RecvOpen stripped setuid/setgid/sticky bits when the
* client sent creat 04755 (setuid + rwxr-xr-x). The expected base mode is
* 0755 with the process umask applied, captured before any threads start. */
static int checkCreatStripsSpecialBits(void)
{
WSTAT_T st;
unsigned int expectedMode;
expectedMode = (unsigned int)(0755 & ~sftpTestUmask);
WMEMSET(&st, 0, sizeof(WSTAT_T));
if (WSTAT(NULL, "test-creat-special", &st) != 0) {
fprintf(stderr, "stat test-creat-special failed\n");
return 1;
}
if (st.st_mode & 07000) {
fprintf(stderr,
"WOLFSSH_SFTP_SAFE_MODE (RecvOpen): special bits not stripped: "
"mode=%06o\n", (unsigned)(st.st_mode & 07777));
return 1;
}
if ((st.st_mode & 0777) != expectedMode) {
fprintf(stderr,
"WOLFSSH_SFTP_SAFE_MODE (RecvOpen): unexpected base mode=%06o, "
"want %04o\n", (unsigned)(st.st_mode & 07777), expectedMode);
return 1;
}
return 0;
}
#endif /* !USE_WINDOWS_API && !WOLFSSH_FATFS && !WOLFSSH_ZEPHYR */
#if !defined(NO_WOLFSSH_DIR) && !defined(WOLFSSH_FATFS)
static int checkLlsHasConfigureAc(void)
{
@ -209,8 +271,9 @@ static const SftpTestCmd cmds[] = {
* the files may not exist. */
{ "rm a/configure.ac", NULL },
{ "rmdir a", NULL },
{ "rm test-get", NULL },
{ "rm test-get-2", NULL },
{ "rm test-get", NULL },
{ "rm test-get-2", NULL },
{ "rm test-creat-special", NULL },
/* --- test sequence starts here --- */
{ "mkdir a", NULL },
@ -235,6 +298,16 @@ static const SftpTestCmd cmds[] = {
{ "rename test-get test-get-2", NULL },
{ "rmdir a", NULL },
{ "ls", checkLsHasTestGet2 },
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_FATFS) && \
!defined(WOLFSSH_ZEPHYR)
/* chmod with setuid bit set; checkChmodStripsSpecialBits verifies
* SFTP_SetFileAttributes applied WOLFSSH_SFTP_SAFE_MODE. */
{ "chmod 4777 test-get-2", checkChmodStripsSpecialBits },
/* creat with setuid bit; checkCreatStripsSpecialBits verifies
* wolfSSH_SFTP_RecvOpen applied WOLFSSH_SFTP_SAFE_MODE. */
{ "creat 04755 test-creat-special", checkCreatStripsSpecialBits },
{ "rm test-creat-special", NULL },
#endif
{ "chmod 600 test-get-2", NULL },
{ "rm test-get-2", NULL },
{ "ls -s", checkLsSize },
@ -258,6 +331,7 @@ static const SftpTestCmd cmds[] = {
{ "cd", NULL },
{ "ls", NULL },
{ "chmod", NULL },
{ "creat", NULL },
{ "rmdir", NULL },
{ "rm", NULL },
{ "rename", NULL },
@ -330,6 +404,13 @@ int wolfSSH_SftpTest(int flag)
wolfSSH_Debugging_ON();
#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_FATFS) && \
!defined(WOLFSSH_ZEPHYR)
/* Read umask non-destructively before spawning threads. */
sftpTestUmask = umask(0);
umask(sftpTestUmask);
#endif
argsCount = 0;
args[argsCount++] = ".";
args[argsCount++] = "-1";

View File

@ -133,6 +133,8 @@ struct WS_SFTP_FILEATRB_EX {
#define FILEATRB_PER_DIR 0040000
#define FILEATRB_PER_DEV_BLOCK 0060000
#define FILEATRB_PER_MASK_PERM 0000777
/* Strip setuid/setgid/sticky bits from peer-supplied permissions. */
#define WOLFSSH_SFTP_SAFE_MODE(m) ((m) & FILEATRB_PER_MASK_PERM)
typedef struct WS_SFTP_FILEATRB {
word32 flags;