diff --git a/configure.ac b/configure.ac index 7f295cd1c..b5278ad29 100644 --- a/configure.ac +++ b/configure.ac @@ -148,6 +148,15 @@ AC_LINK_IFELSE( AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no])]) +AC_MSG_CHECKING([for linkat]) +AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include +#include ]], + [[(void)linkat(AT_FDCWD, "x", AT_FDCWD, "y", 0);]])], + [AC_DEFINE([HAVE_LINKAT], [1], [linkat() available and declared]) + AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no])]) + AC_CHECK_DECLS([[pread],[pwrite]],,[unistd.h]) # DEBUG diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 0bc1f1dbd..54e378a87 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -4981,6 +4981,9 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char name[WOLFSSH_MAX_FILENAME]; word32 idx = 0; int ret = WS_SUCCESS; +#ifndef USE_WINDOWS_API + WS_SFTP_FILEATRB atr; +#endif byte type = WOLFSSH_FTP_OK; int rc; @@ -5006,7 +5009,20 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) if (ret == WS_SUCCESS) { #ifndef USE_WINDOWS_API - if (WREMOVE(ssh->fs, name) < 0) + /* REMOVE never takes a directory (draft-ietf-secsh-filexfer-02 6.5) */ + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, name, &atr, 1, ssh->ctx->heap) + == WS_SUCCESS + && (atr.per & FILEATRB_PER_MASK_TYPE) == FILEATRB_PER_DIR) { + WLOG(WS_LOG_SFTP, "Not removing a directory"); + ret = WS_BAD_FILE_E; + } + #ifdef WUNLINK + /* unlink() refuses a directory swapped in after the check */ + else if (WUNLINK(ssh->fs, name) < 0) + #else + else if (WREMOVE(ssh->fs, name) < 0) + #endif #else /* USE_WINDOWS_API */ if (WS_DeleteFileA(name, ssh->ctx->heap) == 0) #endif /* USE_WINDOWS_API */ @@ -5050,6 +5066,52 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } +#ifndef USE_WINDOWS_API +/* Rename without replacing an existing name (draft-ietf-secsh-filexfer-02 + * 6.5). linkat() refuses one atomically; other types, builds without linkat(), + * and filesystems without hard links check first and race a concurrent + * create. + * + * returns WS_SUCCESS on success + */ +static int SFTP_RenameNoReplace(WOLFSSH* ssh, char* old, char* name) +{ + WS_SFTP_FILEATRB atr; + +#ifdef WLINK + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, old, &atr, 1, ssh->ctx->heap) + == WS_SUCCESS + && (atr.per & FILEATRB_PER_MASK_TYPE) == FILEATRB_PER_FILE) { + if (WLINK(ssh->fs, old, name) == 0) { + if (WUNLINK(ssh->fs, old) == 0) { + return WS_SUCCESS; + } + /* undo the link */ + (void)WUNLINK(ssh->fs, name); + return WS_BAD_FILE_E; + } + if (errno == EEXIST) { + WLOG(WS_LOG_SFTP, "Rename target exists"); + return WS_BAD_FILE_E; + } + } +#endif + + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, name, &atr, 1, ssh->ctx->heap) + == WS_SUCCESS) { + WLOG(WS_LOG_SFTP, "Rename target exists"); + return WS_BAD_FILE_E; + } + if (WRENAME(ssh->fs, old, name) < 0) { + return WS_BAD_FILE_E; + } + return WS_SUCCESS; +} +#endif /* USE_WINDOWS_API */ + + /* Handles packet to rename a file * * returns WS_SUCCESS on success @@ -5098,8 +5160,9 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) if (ret == WS_SUCCESS) { #ifndef USE_WINDOWS_API - if (WRENAME(ssh->fs, old, name) < 0) + if (SFTP_RenameNoReplace(ssh, old, name) != WS_SUCCESS) #else /* USE_WINDOWS_API */ + /* MoveFile never replaces */ if (WS_MoveFileA(old, name, ssh->ctx->heap) == 0) #endif /* USE_WINDOWS_API */ { @@ -5212,8 +5275,10 @@ static int SFTP_GetAttributes(void* fs, const char* fileName, return WS_SUCCESS; } - /* handle case of "/" */ - if (sz < 3 && fileName[0] == WS_DELIM && ret == NUF_NOFILE) { + /* handle case of "/" or "/.", not a root-level name such as "/a" */ + if (fileName[0] == WS_DELIM + && (sz == 1 || (sz == 2 && fileName[1] == '.')) + && ret == NUF_NOFILE) { atr->flags |= WOLFSSH_FILEATRB_PERM; atr->per |= 040000; return WS_SUCCESS; @@ -5597,8 +5662,10 @@ static int SFTP_GetAttributes(void* fs, const char* fileName, } /* handle case of "/" */ - /* Calling f_stat for "/" returns FR_INVALID_NAME. So we simulate the result. */ - if (sz < 3 && fileName[0] == WS_DELIM) { + /* Calling f_stat for "/" returns FR_INVALID_NAME. So we simulate the result. + * Only "/" and "/." though; "/a" is a root-level name for f_stat. */ + if (fileName[0] == WS_DELIM + && (sz == 1 || (sz == 2 && fileName[1] == '.'))) { atr->flags |= WOLFSSH_FILEATRB_PERM; atr->per |= 040000; return WS_SUCCESS; diff --git a/tests/regress.c b/tests/regress.c index 76bc7c4d9..ff9a15bd1 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -16021,6 +16021,167 @@ static void TestSftpAppendWritesAtEof(void) wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); } + +/* FXP_RENAME path to newPath, or FXP_REMOVE path when newPath is NULL. */ +static int SftpPathRequest(WOLFSSH* ssh, int reqId, const char* path, + const char* newPath) +{ + byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)]; + word32 idx = 0; + word32 sz = (word32)WSTRLEN(path); + + SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, sz); idx += sz; + if (newPath == NULL) { + return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx); + } + sz = (word32)WSTRLEN(newPath); + SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, newPath, sz); idx += sz; + return wolfSSH_SFTP_RecvRename(ssh, reqId, pkt, idx); +} + +static void SftpWriteTestFile(const char* path, const char* content) +{ + WFILE* file; + + AssertIntEQ(WFOPEN(NULL, &file, path, "wb"), 0); + AssertTrue(file != WBADFILE); + AssertIntEQ((int)WFWRITE(NULL, content, 1, WSTRLEN(content), file), + (int)WSTRLEN(content)); + WFCLOSE(NULL, file); +} + +/* REMOVE refuses a directory, RENAME an existing newpath + * (draft-ietf-secsh-filexfer-02 6.5). */ +static void TestSftpRemoveRenameSemantics(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + int rid = 700; + WSTAT_T st; + char cwd[WOLFSSH_MAX_FILENAME]; + char fileA[64], fileB[64], fileC[64]; + char dirA[64], dirB[64], dirC[64]; +#if defined(WOLFSSH_HAVE_SYMLINK) && !defined(USE_WINDOWS_API) + char lnk[64]; +#endif +#ifndef USE_WINDOWS_API + char roDir[64], roFile[128], roDest[128]; +#endif + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + + WSNPRINTF(fileA, sizeof(fileA), "wolfssh_rm_a_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(fileB, sizeof(fileB), "wolfssh_rm_b_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(fileC, sizeof(fileC), "wolfssh_rm_c_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(dirA, sizeof(dirA), "wolfssh_rm_da_%lu", SFTP_TEST_PID()); + WSNPRINTF(dirB, sizeof(dirB), "wolfssh_rm_db_%lu", SFTP_TEST_PID()); + WSNPRINTF(dirC, sizeof(dirC), "wolfssh_rm_dc_%lu", SFTP_TEST_PID()); + WMEMSET(cwd, 0, sizeof(cwd)); + AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); + + SftpWriteTestFile(fileA, "aaaa"); + SftpWriteTestFile(fileB, "bb"); + (void)WREMOVE(ssh->fs, fileC); + AssertIntEQ(WMKDIR(ssh->fs, dirA, 0755), 0); + AssertIntEQ(WMKDIR(ssh->fs, dirB, 0755), 0); + (void)WRMDIR(ssh->fs, dirC); + + /* REMOVE refuses an empty directory */ + AssertIntEQ(SftpPathRequest(ssh, rid, dirA, NULL), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WSTAT(ssh->fs, dirA, &st), 0); + + /* RENAME refuses an existing file; neither changes */ + AssertIntEQ(SftpPathRequest(ssh, rid, fileA, fileB), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertSftpFileContents(fileA, "aaaa", 4); + AssertSftpFileContents(fileB, "bb", 2); + + /* RENAME refuses an existing empty directory */ + AssertIntEQ(SftpPathRequest(ssh, rid, dirA, dirB), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WSTAT(ssh->fs, dirA, &st), 0); + AssertIntEQ(WSTAT(ssh->fs, dirB, &st), 0); + +#if defined(WOLFSSH_HAVE_SYMLINK) && !defined(USE_WINDOWS_API) + /* a dangling symlink is an existing newpath too */ + WSNPRINTF(lnk, sizeof(lnk), "wolfssh_rm_ln_%lu", SFTP_TEST_PID()); + (void)WREMOVE(ssh->fs, lnk); + AssertIntEQ(symlink(fileC, lnk), 0); + AssertIntEQ(SftpPathRequest(ssh, rid, fileA, lnk), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WLSTAT(ssh->fs, lnk, &st), 0); + AssertTrue(S_ISLNK(st.st_mode)); + AssertSftpFileContents(fileA, "aaaa", 4); + /* REMOVE takes the link, not its target */ + AssertIntEQ(SftpPathRequest(ssh, rid, lnk, NULL), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WLSTAT(ssh->fs, lnk, &st) != 0); + /* a link to a directory is not a directory to REMOVE */ + AssertIntEQ(symlink(dirB, lnk), 0); + AssertIntEQ(SftpPathRequest(ssh, rid, lnk, NULL), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WLSTAT(ssh->fs, lnk, &st) != 0); + AssertIntEQ(WSTAT(ssh->fs, dirB, &st), 0); +#endif + +#ifndef USE_WINDOWS_API + /* a failed move leaves both names as they were: out of a read-only + * directory takes the link rollback, into one the link fallback */ + if (geteuid() != 0) { + WSNPRINTF(roDir, sizeof(roDir), "wolfssh_rm_ro_%lu", + SFTP_TEST_PID()); + WSNPRINTF(roFile, sizeof(roFile), "%s/%s", roDir, fileC); + WSNPRINTF(roDest, sizeof(roDest), "%s/%s", roDir, fileB); + AssertIntEQ(WMKDIR(ssh->fs, roDir, 0755), 0); + SftpWriteTestFile(roFile, "cc"); + AssertIntEQ(chmod(roDir, 0555), 0); + + AssertIntEQ(SftpPathRequest(ssh, rid, roFile, fileC), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertSftpFileContents(roFile, "cc", 2); + AssertTrue(WLSTAT(ssh->fs, fileC, &st) != 0); + + AssertIntEQ(SftpPathRequest(ssh, rid, fileB, roDest), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertSftpFileContents(fileB, "bb", 2); + AssertTrue(WLSTAT(ssh->fs, roDest, &st) != 0); + + AssertIntEQ(chmod(roDir, 0755), 0); + AssertIntEQ(WREMOVE(ssh->fs, roFile), 0); + AssertIntEQ(WRMDIR(ssh->fs, roDir), 0); + } +#endif + + /* RENAME to a free name moves a file and a directory */ + AssertIntEQ(SftpPathRequest(ssh, rid, fileA, fileC), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, fileA, &st) != 0); + AssertSftpFileContents(fileC, "aaaa", 4); + AssertIntEQ(SftpPathRequest(ssh, rid, dirA, dirC), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, dirA, &st) != 0); + AssertIntEQ(WSTAT(ssh->fs, dirC, &st), 0); + + /* REMOVE still removes a file */ + AssertIntEQ(SftpPathRequest(ssh, rid, fileC, NULL), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, fileC, &st) != 0); + + (void)WREMOVE(ssh->fs, fileB); + (void)WRMDIR(ssh->fs, dirB); + (void)WRMDIR(ssh->fs, dirC); + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} #endif /* !NO_WOLFSSH_SERVER && !NO_FILESYSTEM */ #if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ @@ -20771,6 +20932,8 @@ int main(int argc, char** argv) #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) /* FXF_APPEND writes land at EOF regardless of the request offset */ TestSftpAppendWritesAtEof(); + /* REMOVE refuses directories, RENAME refuses an existing newpath */ + TestSftpRemoveRenameSemantics(); #endif #if !defined(NO_WOLFSSH_SERVER) && defined(USE_WINDOWS_API) && \ !defined(NO_FILESYSTEM) diff --git a/wolfssh/port.h b/wolfssh/port.h index 2190b1650..f0c418dcd 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1613,7 +1613,7 @@ extern "C" { #else - #include /* used for rmdir */ + #include /* used for rmdir, linkat, and unlink */ #include /* used for mkdir, stat, and lstat */ #include /* used for remove and rename */ @@ -1622,6 +1622,15 @@ extern "C" { #define WSTAT(fs,p,b) stat((p),(b)) #ifndef USE_OSE_API #define WLSTAT(fs,p,b) lstat((p),(b)) + #define WUNLINK(fs,p) unlink((p)) + #ifdef HAVE_LINKAT + /* SFTP no-replace rename; it reads errno. linkat() without + * AT_SYMLINK_FOLLOW links a symlink itself, where link() may + * follow it. */ + #include + #include + #define WLINK(fs,o,n) linkat(AT_FDCWD, (o), AT_FDCWD, (n), 0) + #endif #else #define WLSTAT(fs,p,b) stat((p),(b)) #endif