Skip to content

scp: fix duplicate file header on send #1128

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

Open
ejohnstown wants to merge 2 commits into
base: master
Choose a base branch
from
Open

scp: fix duplicate file header on send #1128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f6a8e7
scp: fix duplicate file header on send
ejohnstown Jul 23, 2026
c442e4b
scp: reuse GetScpFileName buffer instead of realloc
ejohnstown Jul 30, 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
5 changes: 5 additions & 0 deletions examples/echoserver/echoserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3200,6 +3200,11 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args)

wolfSSH_SetUserAuthResult(ctx, wsUserAuthResult);
wolfSSH_CTX_SetBanner(ctx, echoserverBanner);
#ifdef WOLFSSH_SCP
/* let a test inject a custom scp send callback in place of the default */
if (serverArgs->scp_send != NULL)
wolfSSH_SetScpSend(ctx, serverArgs->scp_send);
#endif
#ifdef WOLFSSH_AGENT
wolfSSH_CTX_set_agent_cb(ctx, wolfSSH_AGENT_DefaultActions, NULL);
#endif
Expand Down
4 changes: 4 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1605,10 +1605,13 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx)
ssh->scpFileBufferSz = 0;
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
ssh->scpFileNameCap = 0;
ssh->scpTimestamp = 0;
ssh->scpATime = 0;
ssh->scpMTime = 0;
ssh->scpRequestType = WOLFSSH_SCP_SINGLE_FILE_REQUEST;
ssh->scpFileHeaderSent = 0;
ssh->scpNoProgress = 0;
ssh->scpIsRecursive = 0;
ssh->scpDirection = WOLFSSH_SCP_DIR_NONE;
ssh->scpDirDepth = 0;
Expand Down Expand Up @@ -1704,6 +1707,7 @@ void SshResourceFree(WOLFSSH* ssh, void* heap)
WFREE(ssh->scpFileName, heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
ssh->scpFileNameCap = 0;
}
if (ssh->scpRecvMsg) {
WFREE(ssh->scpRecvMsg, heap, DYNTYPE_STRING);
Expand Down
168 changes: 118 additions & 50 deletions src/wolfscp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,27 +366,45 @@ static int ScpSourceInit(WOLFSSH* ssh)
WFREE(ssh->scpFileName, ssh->ctx->heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
ssh->scpFileNameCap = 0;
}

ssh->scpFileName = (char*)WMALLOC(DEFAULT_SCP_FILE_NAME_SZ, ssh->ctx->heap,
DYNTYPE_STRING);
if (ssh->scpFileName == NULL)
return WS_MEMORY_E;

ssh->scpFileNameSz = DEFAULT_SCP_FILE_NAME_SZ;
/* The source path uses scpFileName as a fixed-size scratch buffer that the
* send callback fills in, so there is no name in it yet. */
ssh->scpFileNameCap = DEFAULT_SCP_FILE_NAME_SZ;
ssh->scpFileNameSz = 0;
WMEMSET(ssh->scpFileName, 0, DEFAULT_SCP_FILE_NAME_SZ);

/* file buffer */
if (ssh->scpFileBuffer != NULL) {
WFREE(ssh->scpFileBuffer, ssh->ctx->heap, DYNTYPE_BUFFER);
ssh->scpFileBuffer = NULL;
ssh->scpFileBufferSz = 0;
}

ssh->scpFileBuffer = (byte*)WMALLOC(DEFAULT_SCP_BUFFER_SZ, ssh->ctx->heap,
DYNTYPE_BUFFER);
if (ssh->scpFileBuffer == NULL) {
WFREE(ssh->scpFileName, ssh->ctx->heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameCap = 0;
return WS_MEMORY_E;
}

ssh->scpFileBufferSz = DEFAULT_SCP_BUFFER_SZ;
WMEMSET(ssh->scpFileBuffer, 0, DEFAULT_SCP_BUFFER_SZ);

/* reset per-file state so a reused connection starts a fresh transfer */
ssh->scpFileOffset = 0;
ssh->scpBufferedSz = 0;
ssh->scpFileHeaderSent = 0;
ssh->scpNoProgress = 0;

return WS_SUCCESS;
}

Expand Down Expand Up @@ -627,9 +645,12 @@ int DoScpSource(WOLFSSH* ssh)
case SCP_TRANSFER:
WLOG(WS_LOG_DEBUG, scpState, "SCP_TRANSFER");

/* the callback writes the name into scpFileName, so it needs
* the buffer capacity, not the current name length */
ssh->scpConfirm = ssh->ctx->scpSendCb(ssh,
ssh->scpRequestType, ssh->scpBasePath,
ssh->scpFileName, ssh->scpFileNameSz, &(ssh->scpMTime),
ssh->scpFileName, ssh->scpFileNameCap,
&(ssh->scpMTime),
&(ssh->scpATime), &(ssh->scpFileMode),
ssh->scpFileOffset, &(ssh->scpFileSz),
ssh->scpFileBuffer + ssh->scpBufferedSz,
Expand Down Expand Up @@ -673,8 +694,10 @@ int DoScpSource(WOLFSSH* ssh)
ssh->scpBufferedSz += ssh->scpConfirm;
ssh->scpConfirm = WS_SCP_CONTINUE;

/* only send timestamp and file header first time */
if (ssh->scpFileOffset == 0) {
/* send timestamp and file header once per file; keying on
* scpFileOffset would resend them when the callback
* returns 0 bytes on its metadata call */
if (!ssh->scpFileHeaderSent) {
if (ssh->scpTimestamp == 1) {
ssh->scpState = SCP_SEND_TIMESTAMP;
} else {
Expand Down Expand Up @@ -747,51 +770,62 @@ int DoScpSource(WOLFSSH* ssh)
break;
}

ssh->scpFileHeaderSent = 1;
ssh->scpState = SCP_RECEIVE_CONFIRMATION;
ssh->scpNextState = SCP_SEND_FILE;
continue;

case SCP_SEND_FILE:
WLOG(WS_LOG_DEBUG, scpState, "SCP_SEND_FILE");

ret = ScpStreamSend(ssh, ssh->scpFileBuffer,
ssh->scpBufferedSz);
if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
/* ScpStreamSend already drove the worker through any rekey
* or full window; a non-blocking want means the socket is
* not ready. Surface it for the caller to retry without
* closing the file mid-transfer. scpBufferedSz and
* scpFileOffset are preserved for the next call. */
break;
}
if (ret == WS_EXTDATA) {
_DumpExtendedData(ssh);
continue;
}
if (ret < 0) {
#if !defined(NO_FILESYSTEM) && \
!defined(WOLFSSH_SCP_USER_CALLBACKS)
/* if the socket send had a fatal error, try to close any
* open file descriptor before exit */
ScpSendCtx* sendCtx = NULL;
sendCtx = (ScpSendCtx*)wolfSSH_GetScpSendCtx(ssh);
if (sendCtx != NULL) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
/* nothing buffered (send callback returned 0 bytes): skip the
* send so no zero-length CHANNEL_DATA goes on the wire; routing
* below handles the empty buffer */
if (ssh->scpBufferedSz > 0) {
ret = ScpStreamSend(ssh, ssh->scpFileBuffer,
ssh->scpBufferedSz);
if (ret == WS_WANT_READ || ret == WS_WANT_WRITE) {
/* ScpStreamSend already drove the worker through any
* rekey or full window; a non-blocking want means the
* socket is not ready. Surface it for the caller to
* retry without closing the file mid-transfer.
* scpBufferedSz and scpFileOffset are preserved for the
* next call. */
break;
}
if (ret == WS_EXTDATA) {
_DumpExtendedData(ssh);
continue;
}
if (ret < 0) {
#if !defined(NO_FILESYSTEM) && \
!defined(WOLFSSH_SCP_USER_CALLBACKS)
/* if the socket send had a fatal error, try to close any
* open file descriptor before exit */
ScpSendCtx* sendCtx = NULL;
sendCtx = (ScpSendCtx*)wolfSSH_GetScpSendCtx(ssh);
if (sendCtx != NULL) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}
#endif
WLOG(WS_LOG_ERROR, scpError, "failed to send file", ret);
break;
}
#endif
WLOG(WS_LOG_ERROR, scpError, "failed to send file", ret);
break;
}

ssh->scpFileOffset += ret;
if (ret != (int)ssh->scpBufferedSz) {
/* case where not all of buffer was sent */
WMEMMOVE(ssh->scpFileBuffer, ssh->scpFileBuffer + ret,
ssh->scpBufferedSz - ret);
ssh->scpFileOffset += ret;
if (ret > 0) {
/* real forward progress, clear the stall detector */
ssh->scpNoProgress = 0;
}
if (ret != (int)ssh->scpBufferedSz) {
/* case where not all of buffer was sent */
WMEMMOVE(ssh->scpFileBuffer, ssh->scpFileBuffer + ret,
ssh->scpBufferedSz - ret);
}
ssh->scpBufferedSz -= ret;
ret = WS_SUCCESS;
}
ssh->scpBufferedSz -= ret;
ret = WS_SUCCESS;

if (ssh->scpBufferedSz > 0) {
/* There is still file data in the buffer to send,
Expand All @@ -800,6 +834,18 @@ int DoScpSource(WOLFSSH* ssh)
continue;
}
else if (ssh->scpFileOffset < ssh->scpFileSz) {
/* A send callback may hand back 0 bytes once, on the call
* that only fills in metadata. Twice running with file
* data still outstanding means it cannot make progress,
* and looping back to SCP_TRANSFER would spin here with no
* socket I/O at all, so fail instead. */
if (ssh->scpNoProgress) {
WLOG(WS_LOG_ERROR, scpError,
"send callback made no progress", WS_SCP_ABORT);
ret = WS_SCP_ABORT;
break;
}
ssh->scpNoProgress = 1;
ssh->scpState = SCP_TRANSFER;
ssh->scpRequestType = WOLFSSH_SCP_CONTINUE_FILE_TRANSFER;

Expand All @@ -808,6 +854,8 @@ int DoScpSource(WOLFSSH* ssh)
if (ssh->scpIsRecursive) {
ssh->scpFileOffset = 0;
ssh->scpBufferedSz = 0;
ssh->scpFileHeaderSent = 0;
ssh->scpNoProgress = 0;
ssh->scpATime = 0;
ssh->scpMTime = 0;
ssh->scpNextState = SCP_TRANSFER;
Expand Down Expand Up @@ -1225,16 +1273,24 @@ static int GetScpFileName(WOLFSSH* ssh, byte* buf, word32 bufSz,
}
}

if (ssh->scpFileName != NULL) {
WFREE(ssh->scpFileName, ssh->ctx->heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
}
/* reuse the existing allocation when the name plus its terminator
* fits; scpFileNameCap is the allocation size, so this is correct no
* matter what the last name length was */
if (ssh->scpFileName == NULL || ssh->scpFileNameCap <= len) {
if (ssh->scpFileName != NULL) {
WFREE(ssh->scpFileName, ssh->ctx->heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
ssh->scpFileNameCap = 0;
}

ssh->scpFileName = (char*)WMALLOC(len + 1, ssh->ctx->heap,
DYNTYPE_STRING);
if (ssh->scpFileName == NULL)
ret = WS_MEMORY_E;
ssh->scpFileName = (char*)WMALLOC(len + 1, ssh->ctx->heap,
DYNTYPE_STRING);
if (ssh->scpFileName == NULL)
ret = WS_MEMORY_E;
else
ssh->scpFileNameCap = len + 1;
}

if (ret == WS_SUCCESS) {
WMEMCPY(ssh->scpFileName, buf + idx, len);
Expand All @@ -1248,6 +1304,14 @@ static int GetScpFileName(WOLFSSH* ssh, byte* buf, word32 bufSz,
return ret;
}

#ifdef WOLFSSH_TEST_INTERNAL
int wolfSSH_TestScpGetFileName(WOLFSSH* ssh, byte* buf, word32 bufSz,
word32* inOutIdx)
{
return GetScpFileName(ssh, buf, bufSz, inOutIdx);
}
#endif /* WOLFSSH_TEST_INTERNAL */

/* Reads timestamp information (access, modification) from beginning
* of string, expects space to be after each time value:
*
Expand Down Expand Up @@ -1410,10 +1474,12 @@ static int ScpCheckForRename(WOLFSSH* ssh)
}

sz = sz - idx; /* size of file name */
if (ssh->scpFileNameSz < (word32)sz || ssh->scpFileName == NULL) {
if (ssh->scpFileName == NULL || ssh->scpFileNameCap <= (word32)sz) {
if (ssh->scpFileName != NULL) {
WFREE(ssh->scpFileName, ssh->ctx->heap, DYNTYPE_STRING);
ssh->scpFileName = NULL;
ssh->scpFileNameSz = 0;
ssh->scpFileNameCap = 0;
}
ssh->scpFileName = (char*)WMALLOC(sz + 1, ssh->ctx->heap,
DYNTYPE_STRING);
Expand All @@ -1423,6 +1489,7 @@ static int ScpCheckForRename(WOLFSSH* ssh)
ssh->scpBasePath = NULL;
return WS_MEMORY_E;
}
ssh->scpFileNameCap = sz + 1;
ssh->scpFileName[0] = '\0'; /* make sure null terminated for check */
}

Expand Down Expand Up @@ -3024,7 +3091,8 @@ static int ScpProcessEntry(WOLFSSH* ssh, char* fileName, word64* mTime,
DEFAULT_SCP_FILE_NAME_SZ);
WSTRNCPY(fileName, sendCtx->currentDir->dir.lfname,
DEFAULT_SCP_FILE_NAME_SZ);
if (wolfSSH_CleanPath(ssh, filePath, DEFAULT_SCP_FILE_NAME_SZ) < 0) {
if (wolfSSH_CleanPath(ssh, filePath,
DEFAULT_SCP_FILE_NAME_SZ) < 0) {
ret = WS_SCP_ABORT;
}
#elif defined(USE_WINDOWS_API)
Expand Down
Loading
Loading