From 2a4e8c40288128f3fbaef418284117a5653be288 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Thu, 11 Jun 2026 16:06:42 +0900 Subject: [PATCH] wolfscp: fix ExtractFileName for separator-less paths --- src/wolfscp.c | 33 ++++++++++++++--- tests/unit.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 4 ++ 3 files changed, 123 insertions(+), 6 deletions(-) diff --git a/src/wolfscp.c b/src/wolfscp.c index 12ecf8ae7..2113411a2 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -2026,7 +2026,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; @@ -2039,6 +2042,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 */ @@ -3150,26 +3161,36 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest, } #endif /* WOLFSSH_HAVE_SYMLINK */ - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { 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 if (ret != WS_SCP_ABORT) { - /* a rejected symlink root already logged its own reason; - * only note the generic stat failure here */ - WLOG(WS_LOG_ERROR, "scp: error getting file stats, abort"); + } else { ret = WS_SCP_ABORT; } diff --git a/tests/unit.c b/tests/unit.c index e18981ef7..d7e4d1675 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -5021,6 +5021,91 @@ static int test_SftpRecvSizeBoundAccept(void) } #endif /* WOLFSSH_SFTP */ +#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 */ @@ -5255,6 +5340,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", diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 858c9a9bc..19fa55cd7 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1463,6 +1463,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 */