diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 24d593494..2e40eec30 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -357,6 +357,7 @@ static void ShowCommands(void) printf("\n\nCommands :\n"); printf("\tcd change directory\n"); printf("\tchmod change mode\n"); + printf("\tcreat create file with given permissions\n"); printf("\tget pulls file(s) from server\n"); printf("\tlcd 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 " " 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; } } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 0dd7e415e..3b535be81 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -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"); @@ -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); @@ -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 @@ -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 @@ -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 @@ -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) { @@ -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; @@ -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; @@ -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, + 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) { diff --git a/tests/sftp.c b/tests/sftp.c index 781edb78c..2d8d940b7 100644 --- a/tests/sftp.c +++ b/tests/sftp.c @@ -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"; diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index 655c87427..71263b519 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -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;