From 4ee445dd07bc5b0beb32b1601ab2f453f821bcdb Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Tue, 1 Sep 2026 18:46:54 -0400 Subject: [PATCH] Addressed comments --- apps/wolfsshd/wolfsshd.c | 36 ++++++++++++++++++++++++++---------- src/wolfsftp.c | 22 +++++++++------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 7d63422a..8f151405 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -3028,6 +3028,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) @@ -3178,6 +3195,7 @@ static int StartSSHD(int argc, char** argv) #ifndef _WIN32 return WS_FATAL_ERROR; #else + _freeWinArgs(argv, argc, cmdArgs); return; #endif #endif @@ -3191,6 +3209,7 @@ static int StartSSHD(int argc, char** argv) #ifndef _WIN32 return WS_SUCCESS; #else + _freeWinArgs(argv, argc, cmdArgs); return; #endif @@ -3199,6 +3218,7 @@ static int StartSSHD(int argc, char** argv) #ifndef _WIN32 return WS_SUCCESS; #else + _freeWinArgs(argv, argc, cmdArgs); return; #endif } @@ -3331,12 +3351,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 @@ -3517,13 +3535,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 diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 7f4d014b..46325bf5 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -4385,7 +4385,6 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) OVERLAPPED offset; HANDLE fd; DWORD bytesWritten; - LARGE_INTEGER fileSize; int ret = WS_SUCCESS; int rc; int isAppend = 0; @@ -4455,19 +4454,16 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) /* 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), so the - * client-supplied offset must be overridden with the current EOF. */ + * 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) { - if (GetFileSizeEx(fd, &fileSize) == 0) { - WLOG(WS_LOG_SFTP, "Error getting file size for append"); - res = err; - type = WOLFSSH_FTP_FAILURE; - ret = WS_INVALID_STATE_E; - } - else { - offset.Offset = fileSize.LowPart; - offset.OffsetHigh = (DWORD)fileSize.HighPart; - } + offset.Offset = 0xFFFFFFFF; + offset.OffsetHigh = 0xFFFFFFFF; } }