WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

sftp: refuse dir REMOVE and replacing RENAME 

SSH_FXP_REMOVE does not take directories and SSH_FXP_RENAME fails on an
existing newpath (draft-ietf-secsh-filexfer-02 6.5). The non-Windows
handlers check with an lstat first, oldpath before newpath, and REMOVE
unlinks. A regular file renames by linkat() and unlink(), which refuses
an existing name atomically; anything else checks then calls rename().

- configure: probe for linkat()
- port.h: WUNLINK on POSIX, WLINK as linkat() when configure finds it
- FatFs, Nucleus: only "/" and "/." stand in for the root
- regress: test both against files, directories, symlinks and a
  read-only directory

Issue: F-15535, F-15536
pull/1364/merge
John Safranek 2026-10-20 11:03:08 -07:00 committed by philljj
parent 00f3e74e1c
commit d43eb01a66
4 changed files with 308 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
configure.ac
View File

@ -199,6 +199,15 @@ AC_LINK_IFELSE(
AC_MSG_RESULT([yes])],
[AC_MSG_RESULT([no])])
AC_MSG_CHECKING([for linkat])
AC_LINK_IFELSE(
[AC_LANG_PROGRAM([[#include <fcntl.h>
#include <unistd.h>]],
[[(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

92
src/wolfsftp.c
View File

@ -4992,6 +4992,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;
@ -5017,7 +5020,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 */
@ -5064,6 +5080,65 @@ 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;
/* a missing or unreachable oldpath is reported first, with its errno */
WMEMSET(&atr, 0, sizeof(atr));
if (SFTP_GetAttributes(ssh->fs, old, &atr, 1, ssh->ctx->heap)
!= WS_SUCCESS) {
return WS_BAD_FILE_E;
}
#ifdef WLINK
if ((atr.per & FILEATRB_PER_MASK_TYPE) == FILEATRB_PER_FILE) {
if (WLINK(ssh->fs, old, name) == 0) {
int err;
if (WUNLINK(ssh->fs, old) == 0) {
return WS_SUCCESS;
}
/* undo the link, keeping the unlink errno */
err = errno;
(void)WUNLINK(ssh->fs, name);
errno = err;
return WS_BAD_FILE_E;
}
if (errno == EEXIST) {
WLOG(WS_LOG_SFTP, "Rename target exists");
errno = EEXIST;
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");
#ifdef WOLFSSH_SFTP_ERRNO
/* the reply status comes from errno, which the lstat left stale */
errno = EEXIST;
#endif
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
@ -5112,8 +5187,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 */
{
@ -5229,8 +5305,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;
@ -5614,8 +5692,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;

248
tests/regress.c
View File

@ -16584,6 +16584,207 @@ static void TestSftpAppendWritesAtEof(void)
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
}
/* FXP_REMOVE, FXP_STAT, FXP_LSTAT or FXP_OPENDIR path, by type. */
static int SftpPathRequest(WOLFSSH* ssh, byte type, int reqId,
const char* path)
{
byte pkt[WOLFSSH_MAX_FILENAME + UINT32_SZ];
word32 idx = 0;
word32 pathSz = (word32)WSTRLEN(path);
SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, path, pathSz); idx += pathSz;
switch (type) {
case WOLFSSH_FTP_REMOVE:
return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx);
case WOLFSSH_FTP_STAT:
return wolfSSH_SFTP_RecvSTAT(ssh, reqId, pkt, idx);
case WOLFSSH_FTP_LSTAT:
return wolfSSH_SFTP_RecvLSTAT(ssh, reqId, pkt, idx);
#ifndef NO_WOLFSSH_DIR
case WOLFSSH_FTP_OPENDIR:
return wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx);
#endif
default:
return WS_BAD_ARGUMENT;
}
}
/* FXP_RENAME from to. */
static int SftpRenamePath(WOLFSSH* ssh, int reqId, const char* from,
const char* to)
{
byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)];
word32 idx = 0;
word32 sz;
sz = (word32)WSTRLEN(from);
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, from, sz); idx += sz;
sz = (word32)WSTRLEN(to);
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, to, 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];
/* the refused unlink or rename reports why */
#ifdef WOLFSSH_SFTP_ERRNO
word32 roStatus = WOLFSSH_FTP_PERMISSION;
#else
word32 roStatus = WOLFSSH_FTP_FAILURE;
#endif
#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, WOLFSSH_FTP_REMOVE, rid, dirA),
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(SftpRenamePath(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, as FAILURE whatever an
* earlier call left in errno */
#ifdef WOLFSSH_SFTP_ERRNO
errno = ENOENT;
#endif
AssertIntEQ(SftpRenamePath(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(SftpRenamePath(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, WOLFSSH_FTP_REMOVE, rid, lnk),
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, WOLFSSH_FTP_REMOVE, rid, lnk),
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(SftpRenamePath(ssh, rid, roFile, fileC), WS_BAD_FILE_E);
AssertSftpStatusReply(ssh, rid++, roStatus);
AssertSftpFileContents(roFile, "cc", 2);
AssertTrue(WLSTAT(ssh->fs, fileC, &st) != 0);
AssertIntEQ(SftpRenamePath(ssh, rid, fileB, roDest), WS_BAD_FILE_E);
AssertSftpStatusReply(ssh, rid++, roStatus);
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(SftpRenamePath(ssh, rid, fileA, fileC), WS_SUCCESS);
AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK);
AssertTrue(WSTAT(ssh->fs, fileA, &st) != 0);
AssertSftpFileContents(fileC, "aaaa", 4);
AssertIntEQ(SftpRenamePath(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, WOLFSSH_FTP_REMOVE, rid, fileC),
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) && \
@ -17649,51 +17850,6 @@ static void TestSftpOpenDirAllocFailure(void)
#endif /* NO_WOLFSSH_DIR */
#ifdef WOLFSSH_SFTP_ERRNO
/* FXP_REMOVE, FXP_STAT, FXP_LSTAT or FXP_OPENDIR path, by type. */
static int SftpPathRequest(WOLFSSH* ssh, byte type, int reqId,
const char* path)
{
byte pkt[WOLFSSH_MAX_FILENAME + UINT32_SZ];
word32 idx = 0;
word32 pathSz = (word32)WSTRLEN(path);
SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, path, pathSz); idx += pathSz;
switch (type) {
case WOLFSSH_FTP_REMOVE:
return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx);
case WOLFSSH_FTP_STAT:
return wolfSSH_SFTP_RecvSTAT(ssh, reqId, pkt, idx);
case WOLFSSH_FTP_LSTAT:
return wolfSSH_SFTP_RecvLSTAT(ssh, reqId, pkt, idx);
#ifndef NO_WOLFSSH_DIR
case WOLFSSH_FTP_OPENDIR:
return wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx);
#endif
default:
return WS_BAD_ARGUMENT;
}
}
/* FXP_RENAME from to. */
static int SftpRenamePath(WOLFSSH* ssh, int reqId, const char* from,
const char* to)
{
byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)];
word32 idx = 0;
word32 sz;
sz = (word32)WSTRLEN(from);
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, from, sz); idx += sz;
sz = (word32)WSTRLEN(to);
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
WMEMCPY(pkt + idx, to, sz); idx += sz;
return wolfSSH_SFTP_RecvRename(ssh, reqId, pkt, idx);
}
/* A request naming a file that does not exist answers NO_SUCH_FILE, one the
* server may not reach answers PERMISSION_DENIED, and anything else stays
* FAILURE (draft-ietf-secsh-filexfer-02 Section 7). The cause comes from
@ -22277,6 +22433,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)

11
wolfssh/port.h
View File

@ -1613,7 +1613,7 @@ extern "C" {
#else
#include <unistd.h> /* used for rmdir */
#include <unistd.h> /* used for rmdir, linkat, and unlink */
#include <sys/stat.h> /* used for mkdir, stat, and lstat */
#include <stdio.h> /* used for remove and rename */
#include <errno.h> /* used for why a call failed */
@ -1626,6 +1626,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 <errno.h>
#include <fcntl.h>
#define WLINK(fs,o,n) linkat(AT_FDCWD, (o), AT_FDCWD, (n), 0)
#endif
#else
#define WLSTAT(fs,p,b) stat((p),(b))
#endif