scp: fix duplicate file header on send

ejohnstown · ejohnstown · commit e0d32bb247ea · 2026-07-23T14:47:38.000-07:00
- Gate header on new scpFileHeaderSent flag, not scpFileOffset==0
- A send callback returning 0 bytes first no longer re-sends header
- Skip zero-length SCP_SEND_FILE send to avoid empty CHANNEL_DATA
- Reset offset/bufferedSz/flag in ScpSourceInit for connection reuse
- Add test_wolfSSH_SCP_SendZeroFirst regression (func_args scp_send hook)

Issue: ZD-22176
diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c
@@ -3184,6 +3184,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
diff --git a/src/internal.c b/src/internal.c
@@ -1514,6 +1514,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx)
     ssh->scpATime        = 0;
     ssh->scpMTime        = 0;
     ssh->scpRequestType  = WOLFSSH_SCP_SINGLE_FILE_REQUEST;
+    ssh->scpFileHeaderSent = 0;
     ssh->scpIsRecursive  = 0;
     ssh->scpDirection    = WOLFSSH_SCP_DIR_NONE;
     ssh->scpDirDepth     = 0;
diff --git a/src/wolfscp.c b/src/wolfscp.c
@@ -383,6 +383,11 @@ static int ScpSourceInit(WOLFSSH* ssh)
     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;
+
     return WS_SUCCESS;
 }
 
@@ -669,8 +674,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. A send
+                     * callback may return 0 bytes on its first call (metadata
+                     * now, data next), so key on the flag not scpFileOffset. */
+                    if (!ssh->scpFileHeaderSent) {
                         if (ssh->scpTimestamp == 1) {
                             ssh->scpState = SCP_SEND_TIMESTAMP;
                         } else {
@@ -743,51 +750,58 @@ 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 != (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,
@@ -804,6 +818,7 @@ int DoScpSource(WOLFSSH* ssh)
                     if (ssh->scpIsRecursive) {
                         ssh->scpFileOffset = 0;
                         ssh->scpBufferedSz = 0;
+                        ssh->scpFileHeaderSent = 0;
                         ssh->scpATime = 0;
                         ssh->scpMTime = 0;
                         ssh->scpNextState = SCP_TRANSFER;
diff --git a/tests/api.c b/tests/api.c
@@ -3307,12 +3307,146 @@ static void test_wolfSSH_SCP_ReKey_ToServer_NonBlock(void)
     scp_rekey_test(1, 1);
 }
 
+/* A send callback that returns 0 bytes on its first
+ * WOLFSSH_SCP_SINGLE_FILE_REQUEST (metadata now, data on the following call)
+ * must not make the server send the file header twice. The offset is static
+ * because the one-shot echoserver runs a single transfer. */
+static byte scpZeroFirstData[SCP_REKEY_FILE_SZ];
+static word32 scpZeroFirstOffset;
+
+static int scpSendZeroFirst(WOLFSSH* ssh, int state, const char* peerRequest,
+        char* fileName, word32 fileNameSz, word64* mTime, word64* aTime,
+        int* fileMode, word32 fileOffset, word32* totalFileSz,
+        byte* buf, word32 bufSz, void* ctx)
+{
+    word32 remain, n;
+
+    (void)ssh;
+    (void)peerRequest;
+    (void)fileOffset;
+    (void)ctx;
+
+    switch (state) {
+        case WOLFSSH_SCP_NEW_REQUEST:
+            return WS_SUCCESS;
+
+        case WOLFSSH_SCP_SINGLE_FILE_REQUEST:
+            /* fill metadata, but hand back zero data bytes on this first call */
+            WSTRNCPY(fileName, "scp_hdr_zero.txt", fileNameSz);
+            if (totalFileSz != NULL) *totalFileSz = SCP_REKEY_FILE_SZ;
+            if (mTime != NULL)       *mTime = 0;
+            if (aTime != NULL)       *aTime = 0;
+            if (fileMode != NULL)    *fileMode = 0644;
+            scpZeroFirstOffset = 0;
+            return 0;
+
+        case WOLFSSH_SCP_CONTINUE_FILE_TRANSFER:
+            remain = SCP_REKEY_FILE_SZ - scpZeroFirstOffset;
+            if (remain == 0)
+                return WS_SCP_COMPLETE;
+            n = (remain < bufSz) ? remain : bufSz;
+            WMEMCPY(buf, scpZeroFirstData + scpZeroFirstOffset, n);
+            scpZeroFirstOffset += n;
+            return (int)n;
+
+        default:
+            return WS_SCP_ABORT;
+    }
+}
+
+static void test_wolfSSH_SCP_SendZeroFirst(void)
+{
+    func_args ser;
+    tcp_ready ready;
+    int argsCount;
+    int ret;
+    word32 i;
+    WS_SOCKET_T clientFd;
+#ifdef USE_WINDOWS_API
+    DWORD rcvTimeout = 20000;
+#else
+    struct timeval rcvTimeout;
+#endif
+    const char* args[6];
+    WOLFSSH_CTX* ctx = NULL;
+    WOLFSSH*     ssh = NULL;
+    const char* srcName  = "./scp_hdr_src.txt";
+    const char* fromName = "./scp_hdr_from.txt";
+    char srcBuf[32];
+    char fromBuf[32];
+    char cmd[64];
+    THREAD_TYPE serThread;
+
+    WSTRNCPY(srcBuf, srcName, sizeof(srcBuf));
+    WSTRNCPY(fromBuf, fromName, sizeof(fromBuf));
+
+    for (i = 0; i < SCP_REKEY_FILE_SZ; i++)
+        scpZeroFirstData[i] = (byte)((i * 7 + 1) & 0xff);
+    /* The on-disk file only satisfies the server's base-path parsing; the
+     * custom callback supplies the actual bytes, so a duplicated header shows
+     * up as a content mismatch below rather than a missing file. */
+    AssertIntEQ(scpWriteTestFile(srcName, scpZeroFirstData, SCP_REKEY_FILE_SZ),
+            0);
+
+    WMEMSET(&ser, 0, sizeof(func_args));
+    argsCount = 0;
+    args[argsCount++] = ".";
+    args[argsCount++] = "-1";
+    args[argsCount++] = "-p";
+    args[argsCount++] = "0";
+    ser.argv     = (char**)args;
+    ser.argc     = argsCount;
+    ser.signal   = &ready;
+    ser.scp_send = scpSendZeroFirst;
+    InitTcpReady(ser.signal);
+    ThreadStart(echoserver_test, (void*)&ser, &serThread);
+    WaitTcpReady(&ready);
+
+    WSNPRINTF(cmd, sizeof(cmd), "scp -f %s", srcName);
+    scp_client_connect(&ctx, &ssh, ready.port, cmd);
+    AssertNotNull(ctx);
+    AssertNotNull(ssh);
+
+    /* bound the recv so a regression fails the match assert below, not CI */
+    clientFd = wolfSSH_get_fd(ssh);
+#ifdef USE_WINDOWS_API
+    (void)setsockopt(clientFd, SOL_SOCKET, SO_RCVTIMEO,
+            (const char*)&rcvTimeout, sizeof(rcvTimeout));
+#else
+    rcvTimeout.tv_sec = 20;
+    rcvTimeout.tv_usec = 0;
+    (void)setsockopt(clientFd, SOL_SOCKET, SO_RCVTIMEO,
+            &rcvTimeout, sizeof(rcvTimeout));
+#endif
+
+    ret = wolfSSH_SCP_from(ssh, srcBuf, fromBuf);
+    AssertIntEQ(ret, WS_SUCCESS);
+
+    ret = wolfSSH_shutdown(ssh);
+    (void)ret;
+
+    clientFd = wolfSSH_get_fd(ssh);
+    WCLOSESOCKET(clientFd);
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+    ThreadJoin(serThread);
+    FreeTcpReady(&ready);
+
+    /* a duplicate header would corrupt the stream; an exact match proves the
+     * header was sent once */
+    AssertIntEQ(scpFilesMatch(fromName, scpZeroFirstData, SCP_REKEY_FILE_SZ), 0);
+
+    WREMOVE(NULL, srcName);
+    WREMOVE(NULL, fromName);
+}
+
 #else /* WOLFSSH_SCP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED &&
        * !NO_FILESYSTEM && !WOLFSSH_SCP_USER_CALLBACKS && !WOLFSSH_ZEPHYR */
 static void test_wolfSSH_SCP_ReKey(void) { ; }
 static void test_wolfSSH_SCP_ReKey_NonBlock(void) { ; }
 static void test_wolfSSH_SCP_ReKey_ToServer(void) { ; }
 static void test_wolfSSH_SCP_ReKey_ToServer_NonBlock(void) { ; }
+static void test_wolfSSH_SCP_SendZeroFirst(void) { ; }
 #endif
 
 
@@ -4346,6 +4480,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
     test_wolfSSH_SCP_ReKey_NonBlock();
     test_wolfSSH_SCP_ReKey_ToServer();
     test_wolfSSH_SCP_ReKey_ToServer_NonBlock();
+    test_wolfSSH_SCP_SendZeroFirst();
 
     /* SFTP tests */
     test_wolfSSH_SFTP_SendReadPacket();
diff --git a/wolfssh/internal.h b/wolfssh/internal.h
@@ -989,6 +989,7 @@ struct WOLFSSH {
     word32 scpFileBufferSz;       /* size of transfer buffer, octets */
     word32 scpFileOffset;         /* current offset into file transfer */
     word32 scpBufferedSz;         /* bytes buffered to send to peer */
+    byte   scpFileHeaderSent;     /* file header already sent for current file */
     word32 scpDirDepth;           /* SCP nested NEW_DIR depth below base path */
 #ifdef WOLFSSL_NUCLEUS
     int    scpFd;            /* SCP receive callback context handle */
diff --git a/wolfssh/test.h b/wolfssh/test.h
@@ -41,6 +41,12 @@
 /*#include <wolfssh/error.h>*/
 #endif
 
+#ifdef WOLFSSH_SCP
+    /* for WS_CallbackScpSend used in func_args; test.h is included by sources
+     * (e.g. testsuite.c) that do not otherwise pull in wolfscp.h */
+    #include <wolfssh/wolfscp.h>
+#endif
+
 #ifdef USE_WINDOWS_API
     #ifndef _WIN32_WCE
         #include <process.h>
@@ -857,6 +863,10 @@ typedef struct func_args {
     /* callback for example sftp client commands instead of WFGETS */
     WS_CallbackSftpCommand sftp_cb;
 #endif
+#ifdef WOLFSSH_SCP
+    /* override the server's default scp send callback (test injection) */
+    WS_CallbackScpSend scp_send;
+#endif
 } func_args;
 
 
