Skip to content

Fix Windows SFTP open flags and wolfsshd -D argument parsing(f8827) #1173

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

Merged
ejohnstown merged 4 commits into from
Sep 2, 2026
+584 −77
Merged

Fix Windows SFTP open flags and wolfsshd -D argument parsing(f8827) #1173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5c657e
fix Windows SFTP
miyazakh Aug 13, 2026
4c9133e
Addressed review comments
miyazakh Aug 20, 2026
5fdde0b
Addressed review comments
miyazakh Aug 25, 2026
6a95e1b
Addressed comments
miyazakh Sep 1, 2026
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
77 changes: 49 additions & 28 deletions apps/wolfsshd/wolfsshd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2846,6 +2846,23 @@ static char* _convertHelper(WCHAR* in, void* heap) {
return ret;
}

/* free the argv/cmdArgs buffers built from the wide command line. Safe to
* call on partially initialized state: NULL argv or cmdArgs is ignored. */
static void _freeWinArgs(char** argv, DWORD argc, LPWSTR* cmdArgs)
{
DWORD z;

if (argv != NULL) {
for (z = 0; z < argc; z++) {
WFREE(argv[z], NULL, DYNTYPE_SSHD);
}
WFREE(argv, NULL, DYNTYPE_SSHD);
}
if (cmdArgs != NULL) {
LocalFree(cmdArgs);
}
}

static void StartSSHD(DWORD argc, LPTSTR* wargv)
#else
static int StartSSHD(int argc, char** argv)
Expand Down Expand Up @@ -2881,11 +2898,12 @@ static int StartSSHD(int argc, char** argv)
if (cmdArgs == NULL) {
ret = WS_FATAL_ERROR;
}
argc = cmdArgC;

if (ret == WS_SUCCESS) {
for (i = 0; i < argc; i++) {
if (WSTRCMP((char*)(cmdArgs[i]), "-D") == 0) {
for (i = 0; i < (DWORD)cmdArgC; i++) {
/* cmdArgs entries are wide strings (CommandLineToArgvW); compare
* as such instead of reinterpreting as narrow char data. */
if (wcscmp(cmdArgs[i], L"-D") == 0) {
Comment thread
miyazakh marked this conversation as resolved.
isDaemon = 0;
}
}
Expand All @@ -2894,24 +2912,27 @@ static int StartSSHD(int argc, char** argv)
if (isDaemon) {
/* Set the logging to go to OutputDebugString */
wolfSSH_SetLoggingCb(ServiceDebugCb);
}

if (ret == WS_SUCCESS) {
/* we want the arguments to be normal char strings not wchar_t */
argv = (char**)WMALLOC(argc * sizeof(char*), NULL, DYNTYPE_SSHD);
if (argv == NULL) {
ret = WS_MEMORY_E;
}
else {
unsigned int z;
for (z = 0; z < argc; z++) {
argv[z] = _convertHelper(cmdArgs[z], NULL);
}
if (ret == WS_SUCCESS) {
/* Rebuild argv from cmdArgs, not the caller's wargv: wargv is
* narrow data when called from main(), so it may not be a real
* wide string. cmdArgs is always correct either way. */
argc = (DWORD)cmdArgC;

/* we want the arguments to be normal char strings not wchar_t */
argv = (char**)WMALLOC(argc * sizeof(char*), NULL, DYNTYPE_SSHD);
Comment thread
miyazakh marked this conversation as resolved.
if (argv == NULL) {
ret = WS_MEMORY_E;
}
else {
unsigned int z;
for (z = 0; z < argc; z++) {
argv[z] = _convertHelper(cmdArgs[z], NULL);
}
}
}
else {
argv = (char**)wargv;
}
(void)wargv;
#endif

signal(SIGINT, interruptCatch);
Expand All @@ -2928,7 +2949,8 @@ static int StartSSHD(int argc, char** argv)
}
}

while ((ch = mygetopt(argc, argv, "?f:p:h:dDE:o:t")) != -1) {
while (ret == WS_SUCCESS &&
(ch = mygetopt(argc, argv, "?f:p:h:dDE:o:t")) != -1) {
switch (ch) {
case 'f':
configFile = myoptarg;
Expand Down Expand Up @@ -2991,6 +3013,7 @@ static int StartSSHD(int argc, char** argv)
#ifndef _WIN32
return WS_FATAL_ERROR;
#else
_freeWinArgs(argv, argc, cmdArgs);
return;
#endif
#endif
Expand All @@ -3004,6 +3027,7 @@ static int StartSSHD(int argc, char** argv)
#ifndef _WIN32
return WS_SUCCESS;
#else
_freeWinArgs(argv, argc, cmdArgs);
return;
#endif

Expand All @@ -3012,6 +3036,7 @@ static int StartSSHD(int argc, char** argv)
#ifndef _WIN32
return WS_SUCCESS;
#else
_freeWinArgs(argv, argc, cmdArgs);
return;
#endif
}
Expand Down Expand Up @@ -3144,12 +3169,10 @@ static int StartSSHD(int argc, char** argv)
if (SetServiceStatus(serviceStatusHandle, &serviceStatus) == FALSE) {
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Issue updating service status");
}
_freeWinArgs(argv, argc, cmdArgs);
return;
}
}
if (cmdArgs != NULL) {
LocalFree(cmdArgs);
}
}
#endif

Expand Down Expand Up @@ -3330,13 +3353,11 @@ static int StartSSHD(int argc, char** argv)
}

#ifdef _WIN32
if (isDaemon) { /* free up temporary memory used for conversion of args from wchar_t */
unsigned int z;
for (z = 0; z < argc; z++) {
WFREE(argv[z], NULL, DYNTYPE_SSHD);
}
WFREE(argv, NULL, DYNTYPE_SSHD);
}
/* free up temporary memory used for conversion of args from wchar_t.
* Not gated on isDaemon: argv/cmdArgs are allocated in both daemon and
* -D modes. _freeWinArgs() tolerates NULL argv/cmdArgs from an early
* failure, and argc tracks argv's length once it is allocated. */
_freeWinArgs(argv, argc, cmdArgs);
#else
return 0;
#endif
Expand Down
92 changes: 67 additions & 25 deletions src/wolfsftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ static int SFTP_AddFileHandle(WOLFSSH* ssh,
#else
WFD fd,
#endif
const char* fileName, word32 id[2]);
const char* fileName, word32 id[2], int isAppend);
static int SFTP_RemoveFileHandle(WOLFSSH* ssh, word32 id[2]);
static int SFTP_FileHandleCapped(WOLFSSH* ssh);
#endif /* !NO_WOLFSSH_SERVER */
Expand Down Expand Up @@ -2327,6 +2327,35 @@ static void SFTP_HandleIdNext(WOLFSSH* ssh, word32 id[2])

#endif /* !NO_WOLFSSH_SERVER */

#ifdef USE_WINDOWS_API
/* dwCreationDisposition takes one enumerated value, not a bitmask, so
* resolve CREAT/EXCL/TRUNC to a single disposition here. */
static DWORD SFTP_WinCreationDisp(word32 reason)
{
DWORD disp;

if (reason & WOLFSSH_FXF_CREAT) {
if (reason & WOLFSSH_FXF_EXCL)
disp = CREATE_NEW;
else if (reason & WOLFSSH_FXF_TRUNC)
disp = CREATE_ALWAYS;
else
disp = OPEN_ALWAYS;
}
else {
/* TRUNCATE_EXISTING requires GENERIC_WRITE in dwDesiredAccess or
* CreateFile() fails with ERROR_INVALID_PARAMETER; without WRITE
* there is no way to truncate, so fall back to OPEN_EXISTING. */
if ((reason & WOLFSSH_FXF_TRUNC) && (reason & WOLFSSH_FXF_WRITE))
disp = TRUNCATE_EXISTING;
Comment thread
miyazakh marked this conversation as resolved.
else
disp = OPEN_EXISTING;
}

return disp;
}
#endif /* USE_WINDOWS_API */

/* Handles packet to open a file
*
* returns WS_SUCCESS on success
Expand Down Expand Up @@ -2505,7 +2534,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
/* Generate unique file handle ID and add to tracking list */
SFTP_HandleIdNext(ssh, id);

if ((ret = SFTP_AddFileHandle(ssh, fd, dir, id)) != WS_SUCCESS) {
if ((ret = SFTP_AddFileHandle(ssh, fd, dir, id,
(reason & WOLFSSH_FXF_APPEND) ? 1 : 0)) != WS_SUCCESS) {
WLOG(WS_LOG_SFTP, "Unable to store handle");
res = ier;
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
Expand Down Expand Up @@ -2650,25 +2680,14 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
}
#endif

if (reason & WOLFSSH_FXF_READ) {
if (reason & WOLFSSH_FXF_READ)
desiredAccess |= GENERIC_READ;
creationDisp |= OPEN_EXISTING;
}
if (reason & WOLFSSH_FXF_WRITE) {
if (reason & WOLFSSH_FXF_WRITE)
desiredAccess |= GENERIC_WRITE;
if (reason & WOLFSSH_FXF_CREAT) {
if (reason & WOLFSSH_FXF_TRUNC)
creationDisp = CREATE_ALWAYS;
else
creationDisp = OPEN_ALWAYS;
}
#if 0
if (reason & WOLFSSH_FXF_EXCL)
creationDisp |= CREATE_NEW;
if (reason & WOLFSSH_FXF_APPEND)
desiredAccess |= FILE_APPEND_DATA;
#endif
}
if (reason & WOLFSSH_FXF_APPEND)
desiredAccess |= FILE_APPEND_DATA;
Comment thread
miyazakh marked this conversation as resolved.
Comment thread
miyazakh marked this conversation as resolved.

creationDisp = SFTP_WinCreationDisp(reason);
Comment thread
miyazakh marked this conversation as resolved.

#if 0
/* if file permissions not set then use default */
Expand Down Expand Up @@ -2698,7 +2717,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
/* Generate unique file handle ID and add to tracking list */
SFTP_HandleIdNext(ssh, id);

if (SFTP_AddFileHandle(ssh, fileHandle, dir, id) != WS_SUCCESS) {
if (SFTP_AddFileHandle(ssh, fileHandle, dir, id,
(reason & WOLFSSH_FXF_APPEND) ? 1 : 0) != WS_SUCCESS) {
WLOG(WS_LOG_SFTP, "Unable to store handle");
res = ier;
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
Expand Down Expand Up @@ -2767,6 +2787,7 @@ struct WS_FILE_LIST {
char* fileName; /* cleaned full path of the open file */
word32 id[2]; /* handle ID */
struct WS_FILE_LIST* next;
byte isAppend:1; /* WOLFSSH_FXF_APPEND was requested at open */
};

#ifndef NO_WOLFSSH_DIR
Expand Down Expand Up @@ -4089,7 +4110,7 @@ static int SFTP_AddFileHandle(WOLFSSH* ssh,
#else
WFD fd,
#endif
const char* fileName, word32 id[2])
const char* fileName, word32 id[2], int isAppend)
{
WS_FILE_LIST* cur = NULL;
char* fileNameCopy = NULL;
Expand Down Expand Up @@ -4130,6 +4151,7 @@ static int SFTP_AddFileHandle(WOLFSSH* ssh,
cur->fileName = fileNameCopy;
cur->id[0] = id[0];
cur->id[1] = id[1];
cur->isAppend = (isAppend != 0) ? 1 : 0;
cur->next = ssh->fileList;
ssh->fileList = cur;

Expand Down Expand Up @@ -4350,6 +4372,7 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
DWORD bytesWritten;
int ret = WS_SUCCESS;
int rc;
int isAppend = 0;
word32 idx = 0;

const byte* str;
Expand Down Expand Up @@ -4397,6 +4420,7 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
}
else {
fd = fileEntry->fd;
isAppend = fileEntry->isAppend;
}
}
}
Expand All @@ -4413,6 +4437,22 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
}
offset.Offset = (DWORD)strSz;

/* WOLFSSH_FXF_APPEND was requested at open: FILE_APPEND_DATA alone
* does not force writes to EOF once the handle also carries
* FILE_WRITE_DATA (granted implicitly by GENERIC_WRITE). Resolving
* EOF ourselves with GetFileSizeEx() and writing at that offset is a
* non-atomic read-modify-write: the file is shared FILE_SHARE_WRITE,
* so concurrent appenders would resolve the same offset and overwrite
* each other. Setting both OVERLAPPED offset fields to 0xFFFFFFFF
* tells WriteFile() to append atomically at end of file, matching the
* POSIX O_APPEND path. */
if (isAppend) {
offset.Offset = 0xFFFFFFFF;
offset.OffsetHigh = 0xFFFFFFFF;
}
}

if (ret == WS_SUCCESS) {
/* get length to be written */
if (GetStringRef(&strSz, &str, data, maxSz, &idx) != WS_SUCCESS) {
return WS_BUFFER_E;
Expand Down Expand Up @@ -6331,8 +6371,7 @@ int wolfSSH_SFTP_RecvFSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)

#endif /* _WIN32_WCE */

#if defined(WOLFSSH_TEST_INTERNAL) && !defined(USE_WINDOWS_API) && \
!defined(NO_FILESYSTEM)
#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_FILESYSTEM)
/* Test-only plumbing for the forged-handle regression test in tests/regress.c.
*
* The SFTP request handlers buffer their status/handle reply into ssh->recvState
Expand Down Expand Up @@ -6415,10 +6454,12 @@ int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh)
}
#endif /* NO_WOLFSSH_DIR */

#ifndef USE_WINDOWS_API
/* Close the underlying descriptor of the head tracked file handle out of band,
* leaving the node in the list with a now-stale fd. The next RecvClose on that
* handle will see its close() fail, exercising the path that must still drop
* the handle from the tracking list. Returns WS_SUCCESS if a node was found. */
* the handle from the tracking list. Returns WS_SUCCESS if a node was found.
* Not provided for Windows, where fd is a HANDLE, not a WCLOSE-able fd. */
int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh)
{
if (ssh == NULL || ssh->fileList == NULL) {
Expand All @@ -6431,7 +6472,8 @@ int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh)
#endif
return WS_SUCCESS;
}
#endif /* WOLFSSH_TEST_INTERNAL && !USE_WINDOWS_API && !NO_FILESYSTEM */
#endif /* !USE_WINDOWS_API */
#endif /* WOLFSSH_TEST_INTERNAL && !NO_FILESYSTEM */

#endif /* !NO_WOLFSSH_SERVER */

Expand Down
Loading
Loading