wolfscp: fix ExtractFileName for separator-less paths

src/wolfscp.c

@@ -1835,7 +1835,10 @@ static int ExtractFileName(const char* filePath, char* fileName,
         idx++;
     }
 
-    if (separator < 0)
+    /* a path with no separator is a bare file or directory name; the whole
+     * string is then the file name (separator == -1 is handled correctly by
+     * the length math below) */
+    if (pathLen == 0)
         return WS_BAD_ARGUMENT;
 
     fileLen = pathLen - separator - 1;
@@ -1848,6 +1851,14 @@ static int ExtractFileName(const char* filePath, char* fileName,
     return ret;
 }
 
+#ifdef WOLFSSH_TEST_INTERNAL
+int wolfSSH_TestScpExtractFileName(const char* filePath, char* fileName,
+                                   word32 fileNameSz)
+{
+    return ExtractFileName(filePath, fileName, fileNameSz);
+}
+#endif
+
 #if !defined(NO_FILESYSTEM)
 
 /* for porting to systems without errno */
@@ -2837,21 +2848,32 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
 
                 /* first request, keep track of request directory */
                 ret = ScpPushDir(ssh->fs, sendCtx, peerRequest, ssh->ctx->heap);
+                if (ret != WS_SUCCESS) {
+                    WLOG(WS_LOG_ERROR,
+                         "scp: error opening base directory, abort");
+                }
 
                 if (ret == WS_SUCCESS) {
                     /* get file name from request */
                     ret = ExtractFileName(peerRequest, fileName, fileNameSz);
+                    if (ret != WS_SUCCESS) {
+                        WLOG(WS_LOG_ERROR,
+                             "scp: error extracting directory name, abort");
+                    }
                 }
 
                 if (ret == WS_SUCCESS) {
                     ret = GetFileStats(ssh->fs, sendCtx, peerRequest, mTime, aTime,
                                        fileMode);
+                    if (ret != WS_SUCCESS) {
+                        WLOG(WS_LOG_ERROR,
+                             "scp: error getting file stats, abort");
+                    }
                 }
 
                 if (ret == WS_SUCCESS) {
                     ret = WS_SCP_ENTER_DIR;
                 } else {
-                    WLOG(WS_LOG_ERROR, "scp: error getting file stats, abort");
                     ret = WS_SCP_ABORT;
                 }
 

tests/unit.c

@@ -3389,6 +3389,91 @@ static int test_ScpRecvCallback_NewDirChdirFail(void)
 
 #endif /* WOLFSSH_SCP recv callback depth guard test */
 
+#if defined(WOLFSSH_TEST_INTERNAL) && defined(WOLFSSH_SCP) && \
+    !defined(WOLFSSH_SCP_USER_CALLBACKS)
+/* Exercises ExtractFileName, the SCP source helper that splits the leaf name
+ * from a request path. A bare name with no path separator must be accepted
+ * (the whole string is the file name); this is the recursive-source
+ * regression. */
+static int test_ScpExtractFileName(void)
+{
+    char name[64];
+    int ret;
+
+    /* bare name, no separator: whole string is the file name (regression) */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("scp_rk_src", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "scp_rk_src") != 0)
+        return -900;
+
+    /* leading "./" prefix */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("./scp_rk_src", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "scp_rk_src") != 0)
+        return -901;
+
+    /* relative nested path */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("a/b/c", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "c") != 0)
+        return -902;
+
+    /* absolute path */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("/tmp/x/y", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "y") != 0)
+        return -903;
+
+    /* empty path is rejected */
+    ret = wolfSSH_TestScpExtractFileName("", name, sizeof(name));
+    if (ret != WS_BAD_ARGUMENT)
+        return -904;
+
+    /* NULL arguments are rejected */
+    ret = wolfSSH_TestScpExtractFileName(NULL, name, sizeof(name));
+    if (ret != WS_BAD_ARGUMENT)
+        return -905;
+    ret = wolfSSH_TestScpExtractFileName("scp_rk_src", NULL, sizeof(name));
+    if (ret != WS_BAD_ARGUMENT)
+        return -906;
+
+    /* destination too small for name plus null terminator */
+    ret = wolfSSH_TestScpExtractFileName("scp_rk_src", name, 4);
+    if (ret != WS_SCP_PATH_LEN_E)
+        return -907;
+
+    /* bare "." and ".." are accepted as-is (separator-less leaf names); the
+     * recursive walk skips "."/".." directory entries separately */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName(".", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, ".") != 0)
+        return -908;
+
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("..", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "..") != 0)
+        return -909;
+
+    /* trailing separator yields an empty leaf name with success */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("a/b/", name, sizeof(name));
+    if (ret != WS_SUCCESS || WSTRCMP(name, "") != 0)
+        return -910;
+
+    /* exact-fit boundary of the (fileLen + 1 > fileNameSz) size check:
+     * "scp_rk_src" is 10 chars, so 11 just fits and 10 is one too small */
+    WMEMSET(name, 0, sizeof(name));
+    ret = wolfSSH_TestScpExtractFileName("scp_rk_src", name, 11);
+    if (ret != WS_SUCCESS || WSTRCMP(name, "scp_rk_src") != 0)
+        return -911;
+    ret = wolfSSH_TestScpExtractFileName("scp_rk_src", name, 10);
+    if (ret != WS_SCP_PATH_LEN_E)
+        return -912;
+
+    return 0;
+}
+#endif /* WOLFSSH_TEST_INTERNAL && WOLFSSH_SCP && !WOLFSSH_SCP_USER_CALLBACKS */
+
 #endif /* WOLFSSH_TEST_INTERNAL */
 
 /* Error Code And Message Test */
@@ -3559,6 +3644,13 @@ int wolfSSH_UnitTest(int argc, char** argv)
     testResult = testResult || unitResult;
 #endif
 
+#if defined(WOLFSSH_TEST_INTERNAL) && defined(WOLFSSH_SCP) && \
+    !defined(WOLFSSH_SCP_USER_CALLBACKS)
+    unitResult = test_ScpExtractFileName();
+    printf("ScpExtractFileName: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
+    testResult = testResult || unitResult;
+#endif
+
 #ifdef WOLFSSH_TEST_CAPTURING_ALLOCATOR
     unitResult = test_SshResourceFree_zeroesSecrets();
     printf("SshResourceFree_zeroesSecrets: %s\n",

wolfssh/internal.h

@@ -1429,6 +1429,10 @@ enum WS_MessageIdLimits {
     WOLFSSH_API int wolfSSH_TestDoUserAuthRequestEd25519(WOLFSSH* ssh,
             WS_UserAuthData* authData);
 #endif /* !WOLFSSH_NO_ED25519 */
+#if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS)
+    WOLFSSH_API int wolfSSH_TestScpExtractFileName(const char* filePath,
+            char* fileName, word32 fileNameSz);
+#endif /* WOLFSSH_SCP && !WOLFSSH_SCP_USER_CALLBACKS */
 #endif /* WOLFSSH_TEST_INTERNAL */
 
 /* dynamic memory types */