From 16dbbc30e3fa639435645c5065b88a7b666da6e4 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Wed, 3 Jun 2026 13:22:50 +0900 Subject: [PATCH] SFTP path confinement and status-reply refactor --- .github/workflows/network-contention-test.yml | 22 +- .github/workflows/paramiko-sftp-test.yml | 8 +- .github/workflows/sftp-test.yml | 7 +- apps/wolfsshd/test/sshd_large_sftp_test.sh | 23 +- src/wolfsftp.c | 710 +++++++++++------- tests/api.c | 463 ++++++++++++ wolfssh/port.h | 16 + 7 files changed, 974 insertions(+), 275 deletions(-) diff --git a/.github/workflows/network-contention-test.yml b/.github/workflows/network-contention-test.yml index b8ee8db59..cdde41de8 100644 --- a/.github/workflows/network-contention-test.yml +++ b/.github/workflows/network-contention-test.yml @@ -111,13 +111,13 @@ jobs: run: sudo apt-get update && sudo apt-get install -y expect - name: Create large test files + working-directory: ./wolfssh/ run: | - dd if=/dev/urandom of=/tmp/test_1kb.dat bs=1K count=1 - dd if=/dev/urandom of=/tmp/test_2mb.dat bs=1M count=2 - dd if=/dev/urandom of=/tmp/test_10mb.dat bs=1M count=10 - md5sum /tmp/test_*.dat > /tmp/test_checksums.md5 + dd if=/dev/urandom of=test_1kb.dat bs=1K count=1 + dd if=/dev/urandom of=test_2mb.dat bs=1M count=2 + dd if=/dev/urandom of=test_10mb.dat bs=1M count=10 echo "Test files created:" - ls -la /tmp/test_*.dat + ls -la test_*.dat - name: Run extended SFTP file transfer tests working-directory: ./wolfssh/ @@ -148,8 +148,8 @@ jobs: # Test 1KB file transfer echo "Testing 1KB file transfer..." - /tmp/sftp_test.exp /tmp/test_1kb.dat /tmp/recv_1kb.dat - if ! cmp -s /tmp/test_1kb.dat /tmp/recv_1kb.dat; then + /tmp/sftp_test.exp test_1kb.dat /tmp/recv_1kb.dat + if ! cmp -s test_1kb.dat /tmp/recv_1kb.dat; then echo "FAILED: 1KB file integrity check" kill $SERVER_PID 2>/dev/null || true exit 1 @@ -158,8 +158,8 @@ jobs: # Test 2MB file transfer echo "Testing 2MB file transfer..." - /tmp/sftp_test.exp /tmp/test_2mb.dat /tmp/recv_2mb.dat - if ! cmp -s /tmp/test_2mb.dat /tmp/recv_2mb.dat; then + /tmp/sftp_test.exp test_2mb.dat /tmp/recv_2mb.dat + if ! cmp -s test_2mb.dat /tmp/recv_2mb.dat; then echo "FAILED: 2MB file integrity check" kill $SERVER_PID 2>/dev/null || true exit 1 @@ -168,8 +168,8 @@ jobs: # Test 10MB file transfer echo "Testing 10MB file transfer..." - /tmp/sftp_test.exp /tmp/test_10mb.dat /tmp/recv_10mb.dat - if ! cmp -s /tmp/test_10mb.dat /tmp/recv_10mb.dat; then + /tmp/sftp_test.exp test_10mb.dat /tmp/recv_10mb.dat + if ! cmp -s test_10mb.dat /tmp/recv_10mb.dat; then echo "FAILED: 10MB file integrity check" kill $SERVER_PID 2>/dev/null || true exit 1 diff --git a/.github/workflows/paramiko-sftp-test.yml b/.github/workflows/paramiko-sftp-test.yml index d356d5eaa..50d042480 100644 --- a/.github/workflows/paramiko-sftp-test.yml +++ b/.github/workflows/paramiko-sftp-test.yml @@ -167,17 +167,19 @@ jobs: print("Opening SFTP session...") sftp = ssh.open_sftp() + remote_path = 'test_upload.dat' + # Upload test print("Uploading 20MB test file...") start_time = time.time() - sftp.put('/tmp/sftp_upload/test_upload.dat', '/tmp/test_upload.dat') + sftp.put('/tmp/sftp_upload/test_upload.dat', remote_path) upload_time = time.time() - start_time print(f"Upload completed in {upload_time:.2f} seconds") # Download test print("Downloading 20MB test file...") start_time = time.time() - sftp.get('/tmp/test_upload.dat', '/tmp/sftp_download/test_download.dat') + sftp.get(remote_path, '/tmp/sftp_download/test_download.dat') download_time = time.time() - start_time print(f"Download completed in {download_time:.2f} seconds") @@ -197,7 +199,7 @@ jobs: download_path = f'/tmp/sftp_download/stress_test_{i}.dat' start_time = time.time() # Paramiko uses prefetch by default for get() - sftp.get('/tmp/test_upload.dat', download_path) + sftp.get(remote_path, download_path) elapsed = time.time() - start_time # Verify integrity diff --git a/.github/workflows/sftp-test.yml b/.github/workflows/sftp-test.yml index 28cef71cd..780a6e492 100644 --- a/.github/workflows/sftp-test.yml +++ b/.github/workflows/sftp-test.yml @@ -109,7 +109,6 @@ jobs: - name: Run SFTP test working-directory: ./wolfssh/ run: | - mkdir -p /tmp/sftp_test_dir # Create expect script to automate the SFTP client interaction cat > /tmp/sftp_test.exp << 'EOF' #!/usr/bin/expect -f @@ -118,7 +117,7 @@ jobs: expect "Password:" send "upthehill\r" expect "wolfSSH sftp>" - send "put /tmp/test.dat /tmp/sftp_test_dir/test_received.dat\r" + send "put /tmp/test.dat test_received.dat\r" expect "wolfSSH sftp>" send "exit\r" expect eof @@ -131,9 +130,9 @@ jobs: # Run the expect script /tmp/sftp_test.exp - # Verify the files match + # Verify the files match (echoserver's CWD is ./wolfssh/) echo "Verifying file integrity..." - if cmp -s /tmp/test.dat /tmp/sftp_test_dir/test_received.dat; then + if cmp -s /tmp/test.dat test_received.dat; then echo "SFTP Test PASSED: Files match" else echo "SFTP Test FAILED: Files do not match" diff --git a/apps/wolfsshd/test/sshd_large_sftp_test.sh b/apps/wolfsshd/test/sshd_large_sftp_test.sh index 0a3144273..71be8368b 100755 --- a/apps/wolfsshd/test/sshd_large_sftp_test.sh +++ b/apps/wolfsshd/test/sshd_large_sftp_test.sh @@ -16,22 +16,37 @@ if [ -z "$1" ] || [ -z "$2" ]; then exit 1 fi +# wolfSSHd confines SFTP access to the user's home directory, so the remote +# file must live under it. Resolve the same home directory wolfSSHd uses +# (the passwd entry), falling back to $HOME. +HOME_DIR=`getent passwd "$USER" 2>/dev/null | cut -d: -f6` +if [ -z "$HOME_DIR" ]; then + HOME_DIR="$HOME" +fi +# Fail fast with a clear message rather than silently targeting "/" (which the +# now-active SFTP confinement would reject with a non-obvious error) if neither +# the passwd entry nor $HOME yields a usable home directory. +if [ -z "$HOME_DIR" ] || [ "$HOME_DIR" = "/" ]; then + echo "could not resolve a usable home directory for user '$USER'" + exit 1 +fi +REMOTE_FILE="$HOME_DIR/large-random-2.txt" # create a large file with random data (larger than word32 max value) head -c 4400000010 < /dev/random > large-random.txt set -e -echo "$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r `pwd`/large-random-2.txt -h \"$1\" -p \"$2\"" -$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r `pwd`/large-random-2.txt -h "$1" -p "$2" +echo "$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r $REMOTE_FILE -h \"$1\" -p \"$2\"" +$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l large-random.txt -r "$REMOTE_FILE" -h "$1" -p "$2" -cmp large-random.txt large-random-2.txt +cmp large-random.txt "$REMOTE_FILE" RESULT=$? if [ "$RESULT" != "0" ]; then echo "files did not match when compared" exit 1 fi rm -f large-random.txt -rm -f large-random-2.txt +rm -f "$REMOTE_FILE" set +e diff --git a/src/wolfsftp.c b/src/wolfsftp.c index e13adaf55..c82023e8b 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1783,6 +1783,39 @@ int wolfSSH_SFTP_CreateStatus(WOLFSSH* ssh, word32 status, word32 reqId, } +#ifdef WOLFSSH_HAVE_SYMLINK +/* Returns 1 if path is a symbolic link (POSIX) or a reparse point such as a + * symlink or junction (Windows), otherwise 0. A non-existent path (stat + * fails) is reported as not-a-link so that create requests for a new leaf are + * still permitted by the caller. */ +static int SFTP_IsSymlink(const char* path) +{ + int isLink = 0; +#ifdef USE_WINDOWS_API + WIN32_FILE_ATTRIBUTE_DATA attrs; + + /* GetAndCleanPath produces an SFTP-canonical "/C:/..." path. Route it + * through WS_GetFileAttributesExA, which trims the leading slash and uses + * the wide-char API like every other Windows file op here; calling + * GetFileAttributesA on the raw "/C:/..." form would always fail and + * silently disable link detection. GetFileAttributesEx reports the link's + * own attributes (it does not follow the reparse point). */ + if (WS_GetFileAttributesExA(path, &attrs, NULL) != 0 && + (attrs.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) { + isLink = 1; + } +#else + WSTAT_T lst; + + if (WLSTAT(NULL, path, &lst) == 0 && S_ISLNK(lst.st_mode)) { + isLink = 1; + } +#endif + return isLink; +} +#endif /* WOLFSSH_HAVE_SYMLINK */ + + /* * This is a wrapper around the function wolfSSH_RealPath. Since it modifies * the source path value, copy the path from the data stream into a local @@ -1798,14 +1831,111 @@ int wolfSSH_SFTP_CreateStatus(WOLFSSH* ssh, word32 status, word32 reqId, static int GetAndCleanPath(const char* defaultPath, const byte* data, word32 sz, char* s, word32 sSz) { - char r[WOLFSSH_MAX_FILENAME]; + int ret; + word32 dpLen = 0; + char r[WOLFSSH_MAX_FILENAME]; if (sz >= sizeof r) return WS_BUFFER_E; WMEMCPY(r, data, sz); r[sz] = '\0'; - return wolfSSH_RealPath(defaultPath, r, s, sSz); + ret = wolfSSH_RealPath(defaultPath, r, s, sSz); + if (ret == WS_SUCCESS && defaultPath != NULL) { + /* defaultPath is stored in canonical form by + * wolfSSH_SFTP_SetDefaultPath, so a direct prefix compare against the + * canonical resolved request path enforces confinement. */ + dpLen = (word32)WSTRLEN(defaultPath); + /* strip trailing separator(s), but keep a lone "/" as-is */ + while (dpLen > 1 && WOLFSSH_SFTP_IS_DELIM(defaultPath[dpLen - 1])) { + dpLen--; + } + if (dpLen > 1) { + /* resolved path must equal the default path or be within its + * subtree. On Windows the filesystem is case-insensitive and the + * default path is canonicalized to GetCurrentDirectoryA()'s case, + * so compare case-insensitively there to avoid rejecting valid + * in-jail requests that differ only in case. */ +#ifdef USE_WINDOWS_API + if (WSTRNCASECMP(s, defaultPath, dpLen) != 0 || +#else + if (WSTRNCMP(s, defaultPath, dpLen) != 0 || +#endif + (s[dpLen] != '\0' && !WOLFSSH_SFTP_IS_DELIM(s[dpLen]))) { + ret = WS_PERMISSIONS; + } + } + else { + /* default path is "/" - only absolute paths are accepted */ + if (s[0] == '\0' || !WOLFSSH_SFTP_IS_DELIM(s[0])) { + ret = WS_PERMISSIONS; + } + } + } + +#ifdef WOLFSSH_HAVE_SYMLINK + if (ret == WS_SUCCESS && defaultPath != NULL && dpLen > 1) { + /* Defense in depth: the prefix check above only proves the normalized + * path string stays under the jail. Because wolfSSH_RealPath does not + * resolve symlinks, an in-jail symlink pointing outside the jail would + * pass that check and then be followed by the file operation, escaping + * confinement. Walk every path component below the jail root and + * reject if any existing component is a link (in-jail links are + * rejected too, which is the conservative, safe choice). A + * not-yet-created leaf is left to the operation so creates still + * work. */ + word32 i; + word32 sLen = (word32)WSTRLEN(s); + char saved; + + for (i = dpLen; i <= sLen && ret == WS_SUCCESS; i++) { + /* act only at a component boundary (a separator) or the leaf */ + if (i != sLen && !WOLFSSH_SFTP_IS_DELIM(s[i])) { + continue; + } + /* the jail root itself is trusted; only inspect inside the jail */ + if (i <= dpLen) { + continue; + } + saved = s[i]; + s[i] = '\0'; + if (SFTP_IsSymlink(s)) { + ret = WS_PERMISSIONS; + } + s[i] = saved; + } + } +#endif /* WOLFSSH_HAVE_SYMLINK */ + + return ret; +} + + +/* Builds a status packet and queues it for send. + * Returns WS_SUCCESS on success; ssh takes ownership of the allocated buffer. */ +static int SFTP_SendStatus(WOLFSSH* ssh, byte type, int reqId, const char* msg) +{ + byte* out; + word32 outSz = 0; + int ret; + + ret = wolfSSH_SFTP_CreateStatus(ssh, type, reqId, msg, + "English", NULL, &outSz); + if (ret != WS_SIZE_ONLY) { + return WS_FATAL_ERROR; + } + out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); + if (out == NULL) { + return WS_MEMORY_E; + } + ret = wolfSSH_SFTP_CreateStatus(ssh, type, reqId, msg, + "English", out, &outSz); + if (ret != WS_SUCCESS) { + WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); + return WS_FATAL_ERROR; + } + wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); + return WS_SUCCESS; } @@ -1820,12 +1950,12 @@ int wolfSSH_SFTP_RecvRMDIR(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) int ret = 0; char dir[WOLFSSH_MAX_FILENAME]; word32 idx = 0; - byte* out; - word32 outSz = 0; byte type; + int rc; char err[] = "Remove Directory Error"; char suc[] = "Removed Directory"; + char per[] = "Permission denied"; char* res = NULL; if (ssh == NULL) { @@ -1849,36 +1979,29 @@ int wolfSSH_SFTP_RecvRMDIR(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) #endif /* USE_WINDOWS_API */ } - res = (ret != 0)? err : suc; - type = (ret != 0)? WOLFSSH_FTP_FAILURE : WOLFSSH_FTP_OK; - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, - "English", NULL, &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - - if (ret != 0) { - /* @TODO errno holds reason for rmdir failure. Status sent could be - * better if using errno value to send reason i.e. permissions .. */ - WLOG(WS_LOG_SFTP, "Error removing directory %s", dir); - ret = WS_BAD_FILE_E; + if (ret == WS_PERMISSIONS) { + res = per; + type = WOLFSSH_FTP_PERMISSION; + ret = WS_BAD_FILE_E; } else { - ret = WS_SUCCESS; + res = (ret != 0)? err : suc; + type = (ret != 0)? WOLFSSH_FTP_FAILURE : WOLFSSH_FTP_OK; + if (ret != 0) { + WLOG(WS_LOG_SFTP, "Error removing directory %s", dir); + ret = WS_BAD_FILE_E; + } + else { + ret = WS_SUCCESS; + } } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -1895,12 +2018,12 @@ int wolfSSH_SFTP_RecvMKDIR(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) int ret; char dir[WOLFSSH_MAX_FILENAME]; word32 idx = 0; - byte* out; - word32 outSz = 0; byte type; + int rc; char err[] = "Create Directory Error"; char suc[] = "Created Directory"; + char per[] = "Permission denied"; char* res = NULL; if (ssh == NULL) { @@ -1915,61 +2038,59 @@ int wolfSSH_SFTP_RecvMKDIR(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ret = GetAndCleanPath(ssh->sftpDefaultPath, str, strSz, dir, sizeof(dir)); - if (ret != WS_SUCCESS) { + if (ret != WS_SUCCESS && ret != WS_PERMISSIONS) { return ret; } - if (SFTP_ParseAttributes_buffer(ssh, &atr, data, &idx, maxSz) - != WS_SUCCESS) { - return WS_BUFFER_E; - } + if (ret == WS_SUCCESS) { + if (SFTP_ParseAttributes_buffer(ssh, &atr, data, &idx, maxSz) + != WS_SUCCESS) { + return WS_BUFFER_E; + } #ifndef USE_WINDOWS_API #ifndef WOLFSSH_FATFS - { - word32 mode = 040755; - if (atr.flags & WOLFSSH_FILEATRB_PERM) { - mode = atr.per; - } - else { - WLOG(WS_LOG_SFTP, "No permission attribute, using default"); + { + word32 mode = 040755; + if (atr.flags & WOLFSSH_FILEATRB_PERM) { + mode = atr.per; + } + else { + WLOG(WS_LOG_SFTP, "No permission attribute, using default"); + } + ret = WMKDIR(ssh->fs, dir, mode); } - ret = WMKDIR(ssh->fs, dir, mode); - } #else /* WOLFSSH_FATFS */ - /* WMKDIR for FatFS drops mode argument */ - ret = WMKDIR(ssh->fs, dir, 0); + /* WMKDIR for FatFS drops mode argument */ + ret = WMKDIR(ssh->fs, dir, 0); #endif /* WOLFSSH_FATFS */ #else /* USE_WINDOWS_API */ - ret = WS_CreateDirectoryA(dir, ssh->ctx->heap) == 0; + ret = WS_CreateDirectoryA(dir, ssh->ctx->heap) == 0; #endif /* USE_WINDOWS_API */ + } - res = (ret != 0)? err : suc; - type = (ret != 0)? WOLFSSH_FTP_FAILURE : WOLFSSH_FTP_OK; - if (ret != 0) { - WLOG(WS_LOG_SFTP, "Error creating directory %s", dir); + if (ret == WS_PERMISSIONS) { + res = per; + type = WOLFSSH_FTP_PERMISSION; ret = WS_BAD_FILE_E; } else { - ret = WS_SUCCESS; - } - - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, - "English", NULL, &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; + res = (ret != 0)? err : suc; + type = (ret != 0)? WOLFSSH_FTP_FAILURE : WOLFSSH_FTP_OK; + if (ret != 0) { + WLOG(WS_LOG_SFTP, "Error creating directory %s", dir); + ret = WS_BAD_FILE_E; + } + else { + ret = WS_SUCCESS; + } } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -2074,6 +2195,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) word32 idx = 0; int m = 0; int ret = WS_SUCCESS; + int rc; int fdOpened = 0; int outOwnedBySsh = 0; @@ -2086,6 +2208,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char ier[] = "Internal Failure"; char oer[] = "Open File Error"; char naf[] = "Not A File"; + char per[] = "Permission denied"; #ifdef WOLFSSH_STOREHANDLE int handleStored = 0; #endif @@ -2112,8 +2235,15 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) goto cleanup; } - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, strSz, dir, sizeof(dir)) != WS_SUCCESS) { + ret = GetAndCleanPath(ssh->sftpDefaultPath, + str, strSz, dir, sizeof(dir)); + if (ret == WS_PERMISSIONS) { + WLOG(WS_LOG_SFTP, "Creating path for file to open failed"); + rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, per); + ret = (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc; + goto cleanup; + } + else if (ret != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Creating path for file to open failed"); ret = WS_FATAL_ERROR; goto cleanup; @@ -2291,6 +2421,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) DWORD creationDisp = 0; DWORD flagsAndAttrs = 0; int ret = WS_SUCCESS; + int rc; int fileHandleOpened = 0; int outOwnedBySsh = 0; @@ -2302,6 +2433,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char* res = NULL; char ier[] = "Internal Failure"; char oer[] = "Open File Error"; + char per[] = "Permission denied"; #ifdef WOLFSSH_STOREHANDLE int handleStored = 0; #endif @@ -2324,8 +2456,15 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) goto cleanup; } - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, strSz, dir, sizeof(dir)) != WS_SUCCESS) { + ret = GetAndCleanPath(ssh->sftpDefaultPath, + str, strSz, dir, sizeof(dir)); + if (ret == WS_PERMISSIONS) { + WLOG(WS_LOG_SFTP, "Creating path for file to open failed"); + rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, per); + ret = (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc; + goto cleanup; + } + else if (ret != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Creating path for file to open failed"); ret = WS_FATAL_ERROR; goto cleanup; @@ -2477,11 +2616,13 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char dir[WOLFSSH_MAX_FILENAME]; word32 idx = 0; int ret = WS_SUCCESS; + int rc; word32 outSz = sizeof(word32)*2 + WOLFSSH_SFTP_HEADER + UINT32_SZ; byte* out = NULL; word32 id[2]; byte idFlat[sizeof(word32) * 2]; + char per[] = "Permission denied"; if (ssh == NULL) { return WS_BAD_ARGUMENT; @@ -2499,8 +2640,13 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return WS_BUFFER_E; } - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, strSz, dir, sizeof(dir)) < 0) { + ret = GetAndCleanPath(ssh->sftpDefaultPath, + str, strSz, dir, sizeof(dir)); + if (ret == WS_PERMISSIONS) { + rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, per); + return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc; + } + else if (ret != WS_SUCCESS) { return WS_BUFFER_E; } @@ -2579,17 +2725,21 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) { word32 sz; char* dirName; + word32 dirNameSz; word32 idx = 0; HANDLE findHandle; char realName[MAX_PATH]; int isDir = 0; int ret = WS_SUCCESS; + int rc; word32 outSz = sizeof(word32) * 2 + WOLFSSH_SFTP_HEADER + UINT32_SZ; byte* out = NULL; word32 id[2]; byte idFlat[sizeof(word32) * 2]; char name[MAX_PATH]; + char clean[WOLFSSH_MAX_FILENAME]; + char per[] = "Permission denied"; if (ssh == NULL) { return WS_BAD_ARGUMENT; @@ -2614,13 +2764,27 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return WS_BUFFER_E; } + /* Resolve and confine the peer supplied path to sftpDefaultPath, the same + * way the POSIX branch does, so an absolute or UNC path cannot escape the + * configured root. When no default path is set this only normalizes the + * path, preserving the "/" drive listing special case below. */ + ret = GetAndCleanPath(ssh->sftpDefaultPath, data + idx, sz, + clean, sizeof(clean)); + if (ret == WS_PERMISSIONS) { + rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, per); + return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc; + } + else if (ret != WS_SUCCESS) { + return WS_BUFFER_E; + } + /* plus one to make sure is null terminated */ - dirName = (char*)WMALLOC(sz + 1, ssh->ctx->heap, DYNTYPE_BUFFER); + dirNameSz = (word32)WSTRLEN(clean) + 1; + dirName = (char*)WMALLOC(dirNameSz, ssh->ctx->heap, DYNTYPE_BUFFER); if (dirName == NULL) { return WS_MEMORY_E; } - WMEMCPY(dirName, data + idx, sz); - dirName[sz] = '\0'; + WMEMCPY(dirName, clean, dirNameSz); /* Special case in Windows for the root directory above the drives. */ if (dirName[0] == '/' && dirName[1] == 0) { @@ -2645,7 +2809,7 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ssh->driveIdx = 0; } else { - if (sz > MAX_PATH - 2) { + if (dirNameSz - 1 > MAX_PATH - 2) { WLOG(WS_LOG_SFTP, "Path name is too long."); WFREE(dirName, ssh->ctx->heap, DYNTYPE_BUFFER); return WS_FATAL_ERROR; @@ -3700,11 +3864,10 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) { WFD fd; int ret = WS_SUCCESS; + int rc; word32 idx = 0; word32 ofst[2] = {0,0}; - word32 outSz = 0; - byte* out = NULL; const byte* str; word32 strSz; @@ -3760,22 +3923,12 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; - } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } #else /* USE_WINDOWS_API */ @@ -3784,10 +3937,9 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) HANDLE fd; DWORD bytesWritten; int ret = WS_SUCCESS; + int rc; word32 idx = 0; - word32 outSz = 0; - byte* out = NULL; const byte* str; word32 strSz; @@ -3842,22 +3994,12 @@ int wolfSSH_SFTP_RecvWrite(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; - } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -4103,9 +4245,7 @@ int wolfSSH_SFTP_RecvClose(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) const byte* str; word32 idx = 0; int ret = WS_FATAL_ERROR; - - byte* out = NULL; - word32 outSz = 0; + int rc; char* res = NULL; char suc[] = "Closed File"; @@ -4165,22 +4305,12 @@ int wolfSSH_SFTP_RecvClose(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ret = WS_SUCCESS; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } #else /* USE_WINDOWS_API */ @@ -4190,9 +4320,7 @@ int wolfSSH_SFTP_RecvClose(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) const byte* str; word32 idx = 0; int ret = WS_FATAL_ERROR; - - byte* out = NULL; - word32 outSz = 0; + int rc; char* res = NULL; char suc[] = "Closed File"; @@ -4248,22 +4376,12 @@ int wolfSSH_SFTP_RecvClose(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ret = WS_SUCCESS; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; - } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } #endif /* USE_WINDOWS_API */ @@ -4282,12 +4400,11 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) word32 idx = 0; int ret = WS_SUCCESS; - byte* out; - word32 outSz; - byte type = WOLFSSH_FTP_OK; + int rc; char suc[] = "Removed File"; char err[] = "Remove File Error"; + char per[] = "Permission denied"; char* res = suc; if (ssh == NULL) { @@ -4331,27 +4448,22 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } /* Let the client know the results from trying to remove the file */ - if (ret != WS_SUCCESS) { - res = err; + if (ret == WS_PERMISSIONS) { + res = per; + type = WOLFSSH_FTP_PERMISSION; + ret = WS_BAD_FILE_E; + } + else if (ret != WS_SUCCESS) { + res = err; type = WOLFSSH_FTP_FAILURE; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; - } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -4367,14 +4479,14 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) word32 idx = 0; int ret = WS_SUCCESS; - byte* out; - word32 outSz; const byte* str; word32 strSz; byte type = WOLFSSH_FTP_OK; + int rc; char suc[] = "Renamed File"; char err[] = "Rename File Error"; + char per[] = "Permission denied"; char* res = suc; if (ssh == NULL) { @@ -4415,27 +4527,22 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } /* Let the client know the results from trying to rename the file */ - if (ret != WS_SUCCESS) { + if (ret == WS_PERMISSIONS) { + res = per; + type = WOLFSSH_FTP_PERMISSION; + ret = WS_BAD_FILE_E; + } + else if (ret != WS_SUCCESS) { type = WOLFSSH_FTP_FAILURE; res = err; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -5439,6 +5546,11 @@ int wolfSSH_SFTP_RecvSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) byte* out = NULL; word32 outSz = 0; + char per[] = "Permission denied"; + char ser[] = "STAT error"; + char* statusMsg = ser; + byte statusType = WOLFSSH_FTP_FAILURE; + if (ssh == NULL) { return WS_BAD_ARGUMENT; } @@ -5450,13 +5562,19 @@ int wolfSSH_SFTP_RecvSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } /* try to get file attributes and send back to client */ - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, sz, name, sizeof(name)) < 0) { - if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "STAT error", "English", NULL, &outSz) != WS_SIZE_ONLY) { + ret = GetAndCleanPath(ssh->sftpDefaultPath, str, sz, name, sizeof(name)); + if (ret < 0) { + if (ret == WS_PERMISSIONS) { + statusType = WOLFSSH_FTP_PERMISSION; + statusMsg = per; + } + if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId, + statusMsg, "English", NULL, &outSz) != WS_SIZE_ONLY) { return WS_FATAL_ERROR; } - ret = WS_FATAL_ERROR; + /* status is queued below; use WS_BAD_FILE_E to match the other Recv + * handlers' permission/path-failure return code */ + ret = WS_BAD_FILE_E; } if (ret == WS_SUCCESS) { @@ -5465,7 +5583,7 @@ int wolfSSH_SFTP_RecvSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) != WS_SUCCESS) { WLOG(WS_LOG_SFTP, "Unable to get stat of file/directory"); if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "STAT error", "English", NULL, &outSz) != WS_SIZE_ONLY) { + ser, "English", NULL, &outSz) != WS_SIZE_ONLY) { return WS_FATAL_ERROR; } ret = WS_BAD_FILE_E; @@ -5482,8 +5600,8 @@ int wolfSSH_SFTP_RecvSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } if (ret != WS_SUCCESS) { - if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "STAT error", "English", out, &outSz) != WS_SUCCESS) { + if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId, + statusMsg, "English", out, &outSz) != WS_SUCCESS) { WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); return WS_FATAL_ERROR; } @@ -5512,7 +5630,7 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char name[WOLFSSH_MAX_FILENAME]; int ret = WS_SUCCESS; - word32 sz; + word32 sz = 0; word32 strSz; const byte* str; word32 idx = 0; @@ -5520,6 +5638,11 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) byte* out = NULL; word32 outSz = 0; + char per[] = "Permission denied"; + char ser[] = "LSTAT error"; + char* statusMsg = ser; + byte statusType = WOLFSSH_FTP_FAILURE; + if (ssh == NULL) { return WS_BAD_ARGUMENT; } @@ -5530,14 +5653,21 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return WS_BUFFER_E; } - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, strSz, name, sizeof(name)) < 0) { + ret = GetAndCleanPath(ssh->sftpDefaultPath, + str, strSz, name, sizeof(name)); + if (ret < 0) { + if (ret == WS_PERMISSIONS) { + statusType = WOLFSSH_FTP_PERMISSION; + statusMsg = per; + } WLOG(WS_LOG_SFTP, "Unable to clean path"); - if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "LSTAT error", "English", NULL, &outSz) != WS_SIZE_ONLY) { + if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId, + statusMsg, "English", NULL, &outSz) != WS_SIZE_ONLY) { return WS_FATAL_ERROR; } - ret = WS_FATAL_ERROR; + /* status is queued below; use WS_BAD_FILE_E to match the other Recv + * handlers' permission/path-failure return code */ + ret = WS_BAD_FILE_E; } /* try to get file attributes and send back to client */ @@ -5548,7 +5678,7 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) /* tell peer that was not ok */ WLOG(WS_LOG_SFTP, "Unable to get lstat of file/directory"); if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "LSTAT error", "English", NULL, &outSz) != WS_SIZE_ONLY) { + ser, "English", NULL, &outSz) != WS_SIZE_ONLY) { return WS_FATAL_ERROR; } ret = WS_BAD_FILE_E; @@ -5565,8 +5695,8 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } if (ret != WS_SUCCESS) { - if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, - "LSTAT error", "English", out, &outSz) != WS_SUCCESS) { + if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId, + statusMsg, "English", out, &outSz) != WS_SUCCESS) { WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); return WS_FATAL_ERROR; } @@ -5705,17 +5835,16 @@ int wolfSSH_SFTP_RecvSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) WS_SFTP_FILEATRB atr; char name[WOLFSSH_MAX_FILENAME]; int ret = WS_SUCCESS; + int rc; word32 strSz; const byte* str; word32 idx = 0; - byte* out = NULL; - word32 outSz = 0; - char suc[] = "Set Attributes"; char ser[] = "Unable to set attributes error"; char per[] = "Unable to parse attributes error"; + char pdn[] = "Permission denied"; char* res = suc; byte type = WOLFSSH_FTP_OK; @@ -5729,10 +5858,17 @@ int wolfSSH_SFTP_RecvSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return WS_BUFFER_E; } - /* plus one to make sure is null terminated */ - if (GetAndCleanPath(ssh->sftpDefaultPath, - str, strSz, name, sizeof(name)) < 0) { - ret = WS_BUFFER_E; + ret = GetAndCleanPath(ssh->sftpDefaultPath, str, strSz, name, sizeof(name)); + if (ret != WS_SUCCESS) { + if (ret == WS_PERMISSIONS) { + type = WOLFSSH_FTP_PERMISSION; + res = pdn; + } + else { + type = WOLFSSH_FTP_FAILURE; + res = ser; + } + ret = WS_BAD_FILE_E; } if (ret == WS_SUCCESS && @@ -5752,22 +5888,12 @@ int wolfSSH_SFTP_RecvSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ret = WS_BAD_FILE_E; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; - } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -5780,15 +5906,13 @@ int wolfSSH_SFTP_RecvFSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) { WS_SFTP_FILEATRB atr; int ret = WS_SUCCESS; + int rc; WFD fd; word32 strSz; const byte* str; word32 idx = 0; - byte* out = NULL; - word32 outSz = 0; - char suc[] = "Set Attributes"; char ser[] = "Unable to set attributes error"; char per[] = "Unable to parse attributes error"; @@ -5828,22 +5952,12 @@ int wolfSSH_SFTP_RecvFSetSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) ret = WS_BAD_FILE_E; } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL, - &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; - } - out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); - if (out == NULL) { - return WS_MEMORY_E; - } - if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", out, - &outSz) != WS_SUCCESS) { - WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); - return WS_FATAL_ERROR; + /* keep the operation result on success; on a send failure propagate it so + * a status-buffer allocation failure surfaces as WS_MEMORY_E */ + rc = SFTP_SendStatus(ssh, type, reqId, res); + if (rc != WS_SUCCESS) { + return rc; } - - /* set send out buffer, "out" is taken by ssh */ - wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); return ret; } @@ -8251,6 +8365,29 @@ int wolfSSH_SFTP_Close(WOLFSSH* ssh, byte* handle, word32 handleSz) /* Sets the default path that SFTP will start a user in. + * + * Setting a default path other than "/" also confines the session to that + * subtree: requests resolving outside it are rejected with WS_PERMISSIONS + * (see GetAndCleanPath). Because paths are resolved lexically and the result + * cannot prove a link stays in-jail, confinement deliberately rejects ALL + * symbolic links encountered below the default path - including links whose + * target is itself inside the jail (e.g. "current -> releases/v3"). Deploy + * served trees without symlinks, or build with WOLFSSH_NO_SYMLINK_CHECK to + * disable the link check (which also removes the symlink-escape protection). + * + * Note this link check is best-effort, not a hard security boundary: it is a + * time-of-check/time-of-use (TOCTOU) check. GetAndCleanPath inspects each + * path component, then the operation acts on the same path by name in a later, + * separate call. An attacker able to write inside the jail concurrently and + * as the same user the server runs file operations as could swap a validated + * component for a symlink in that window and escape. wolfSSH services a single + * SFTP session's requests serially, so a session cannot race its own + * check-then-use; this requires a separate concurrent writer. For hostile + * multi-tenant deployments, confine the session with an OS-level mechanism + * (e.g. chroot and dropped privileges) and treat this check as defense in + * depth: it reliably blocks static (non-racing) in-jail symlinks but cannot + * close the concurrent-swap race portably (the *at/O_NOFOLLOW primitives the + * full fix needs do not exist across all supported filesystems). * * path NULL-terminated string specifying the default/base path * the SFTP session should start in. If path is NULL, the @@ -8260,21 +8397,88 @@ int wolfSSH_SFTP_Close(WOLFSSH* ssh, byte* handle, word32 handleSz) */ int wolfSSH_SFTP_SetDefaultPath(WOLFSSH* ssh, const char* path) { + int ret = WS_SUCCESS; + word32 canonSz; + const char* canon = NULL; + char* newPath = NULL; + char in[WOLFSSH_MAX_FILENAME]; + char cwd[WOLFSSH_MAX_FILENAME]; + char real[WOLFSSH_MAX_FILENAME]; +#ifdef USE_WINDOWS_API + DWORD cwdLen; +#endif + if (ssh == NULL) return WS_BAD_ARGUMENT; if (path != NULL) { - word32 sftpDefaultPathSz; - sftpDefaultPathSz = (word32)XSTRLEN(path) + 1; - ssh->sftpDefaultPath = (char*)WMALLOC(sftpDefaultPathSz, - ssh->ctx->heap, DYNTYPE_STRING); - if (ssh->sftpDefaultPath == NULL) { - ssh->error = WS_MEMORY_E; - return WS_FATAL_ERROR; + /* Store the default path in canonical form so the SFTP confinement + * check (GetAndCleanPath) can compare it directly against canonicalized + * request paths without re-canonicalizing per request. A relative path + * is resolved against the current working directory; a purely lexical + * canonicalization of e.g. "." would collapse to "/" and leave + * confinement effectively disabled. */ + if (WSTRLEN(path) >= sizeof(in)) { + return WS_BUFFER_E; + } + WSTRNCPY(in, path, sizeof(in)); + in[sizeof(in) - 1] = '\0'; + + if (!WOLFSSH_SFTP_IS_DELIM(in[0]) && + !WOLFSSH_SFTP_IS_WINPATH((word32)WSTRLEN(in), in)) { + /* relative: resolve against the canonicalized working directory */ +#ifdef WOLFSSH_ZEPHYR + WSTRNCPY(cwd, CONFIG_WOLFSSH_SFTP_DEFAULT_DIR, sizeof cwd); +#elif !defined(USE_WINDOWS_API) + if (WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1) == NULL) { + ret = WS_INVALID_PATH_E; + } +#else + /* GetCurrentDirectoryA returns the number of chars written on + * success, or the required size (including the NUL) when the + * buffer is too small; treat zero or an over-long result as a + * failure so a truncated cwd is never canonicalized. */ + cwdLen = GetCurrentDirectoryA(sizeof(cwd) - 1, cwd); + if (cwdLen == 0 || cwdLen >= (DWORD)(sizeof(cwd) - 1)) { + ret = WS_INVALID_PATH_E; + } +#endif + if (ret == WS_SUCCESS) { + cwd[sizeof(cwd) - 1] = '\0'; + ret = wolfSSH_RealPath(NULL, cwd, real, sizeof(real)); + } + if (ret == WS_SUCCESS) { + ret = wolfSSH_RealPath(real, in, cwd, sizeof(cwd)); + canon = cwd; + } + } + else { + ret = wolfSSH_RealPath(NULL, in, real, sizeof(real)); + canon = real; + } + + if (ret == WS_SUCCESS) { + /* Allocate and populate the replacement first, then swap it in, + * freeing the previous path only on success. A failed allocation + * must leave the existing confinement base path intact rather than + * clear it (repeated calls, e.g. wolfsshd setting "/" then the + * user's home dir, also do not leak). */ + canonSz = (word32)WSTRLEN(canon) + 1; + newPath = (char*)WMALLOC(canonSz, ssh->ctx->heap, DYNTYPE_STRING); + if (newPath == NULL) { + ssh->error = WS_MEMORY_E; + ret = WS_FATAL_ERROR; + } + else { + WSTRNCPY(newPath, canon, canonSz); + if (ssh->sftpDefaultPath != NULL) { + WFREE(ssh->sftpDefaultPath, ssh->ctx->heap, DYNTYPE_STRING); + } + ssh->sftpDefaultPath = newPath; + } } - XSTRNCPY(ssh->sftpDefaultPath, path, sftpDefaultPathSz); } - return WS_SUCCESS; + return ret; } diff --git a/tests/api.c b/tests/api.c index 7ef56d3a5..32e23385b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -32,6 +32,14 @@ #include #include +#if defined(WOLFSSH_SFTP) && !defined(NO_WOLFSSH_CLIENT) && \ + !defined(SINGLE_THREADED) && !defined(WOLFSSH_ZEPHYR) && \ + !defined(USE_WINDOWS_API) + /* mkdtemp() for staging unique, per-test out-of-jail SFTP fixtures and + * symlink() for the symlink-escape confinement case */ + #include + #include +#endif #include #include #ifdef WOLFSSH_SCP @@ -1580,10 +1588,462 @@ static void test_wolfSSH_SFTP_ReKey_NonBlock(void) sftp_rekey_test(1); } +static void test_wolfSSH_SFTP_Confinement(void) +{ + func_args ser; + tcp_ready ready; + int argsCount; + WS_SOCKET_T clientFd; + const char* args[10]; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + THREAD_TYPE serThread; + WS_SFTPNAME* ls = NULL; + WS_SFTP_FILEATRB atr; + byte handle[WOLFSSH_MAX_HANDLE]; + word32 handleSz; + int ret; + char curDir[] = "."; + char inJailDir[] = "confine_injail_dir"; + /* The server is confined to its working directory ("."). Every "escape" + * targets an absolute, out-of-jail path. None of these are real system + * files, so a confinement bypass can never damage anything important. */ +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + /* On hosted POSIX, stage real out-of-jail targets under a unique per-test + * temporary directory created with mkdtemp(). Fixed /tmp names race + * against parallel test jobs and against stale paths owned by another + * user, producing false failures unrelated to confinement; a private + * mkdtemp() directory we own avoids both. Because the fixtures exist on + * disk, a confinement bypass for read/stat/delete/rename would actually + * succeed and trip the assertions below - not merely fail with ENOENT. + * escMkdir/escDest are left absent so a leaked create also trips an + * assertion. + * + * The echoserver's jail is its working directory (".", which here is the + * test process's own cwd). If the temp root resolves inside that jail + * (e.g. the suite is run from within /tmp), the "escape" paths would fall + * in-jail and the rejection assertions would invert; that case is detected + * below and skipped. The in-jail positive case is covered by the repeated + * wolfSSH_SFTP_LS(ssh, ".") below, which must succeed after each reject. + * Blocking-mode LS handles any in-progress rekey transparently + * (buffer_read/buffer_send), so no manual rekey-drive helper is needed. */ + char escRoot[] = "/tmp/wolfssh_confine_XXXXXX"; + char escFile[WOLFSSH_MAX_FILENAME]; + char escDir[WOLFSSH_MAX_FILENAME]; + char escMkdir[WOLFSSH_MAX_FILENAME]; + char escDest[WOLFSSH_MAX_FILENAME]; + char jailCwd[WOLFSSH_MAX_FILENAME]; + /* a relative ".." traversal that resolves to the same real out-of-jail + * file as escFile, exercising the post-RealPath containment check on the + * relative-escape path (not just absolute paths) */ + char escRel[WOLFSSH_MAX_FILENAME]; + /* a real sibling directory whose name is the jail's name with a distinctive + * suffix appended directly (no separator); its resolved path matches the + * jail for the full prefix length but the next byte is not a delimiter, so + * only GetAndCleanPath's boundary check (not a plain prefix compare) rejects + * it. The suffix is test-specific so it will not collide with real user + * directories, and it is only created/removed when this run actually staged + * it. Empty when the cwd could not be resolved, the name would truncate, or + * such a directory already exists (the sub-test is then skipped rather than + * touching unrelated data). */ + char escSibling[WOLFSSH_MAX_FILENAME]; +#ifdef WOLFSSH_HAVE_SYMLINK + /* an in-jail symlink pointing at the out-of-jail temp root, and a path + * that traverses it; both must be rejected even though they resolve to an + * in-jail string, since wolfSSH_RealPath does not follow links. Guarded by + * WOLFSSH_HAVE_SYMLINK to match the server-side check's feature gate: on + * POSIX builds that compile the check out (e.g. WOLFSSH_USER_FILESYSTEM) + * the link would be followed as designed, so these assertions must not + * run. */ + char escSymlink[] = "confine_symlink"; + char escSymThru[WOLFSSH_MAX_FILENAME]; +#endif + WFILE* fp = NULL; + int snLen; +#else + /* Zephyr (and Windows, which does not run this via "make check") lack the + * getcwd()/fopen() wrappers to stage out-of-jail files, so fall back to + * non-existent out-of-jail paths. A leaked MKDIR still trips its + * assertion; read/stat/delete bypasses are not detectable here. */ + char escFile[] = "/wolfssh_confine_test_file"; + char escDir[] = "/wolfssh_confine_test_dir"; + char escMkdir[] = "/wolfssh_confine_test_mkdir"; + char escDest[] = "/wolfssh_confine_test_renamed"; +#endif + + /* best effort removal of anything a previous aborted run may have left */ + WRMDIR(NULL, inJailDir); +#if defined(WOLFSSH_ZEPHYR) || defined(USE_WINDOWS_API) + WREMOVE(NULL, escFile); + WRMDIR(NULL, escDir); + WRMDIR(NULL, escMkdir); + WREMOVE(NULL, escDest); +#else + /* Create a private, unique temp directory to hold the out-of-jail + * fixtures, then derive the individual escape paths from it. */ + AssertNotNull(mkdtemp(escRoot)); + + /* If the temp root resolves inside the jail (the test process's cwd), + * the "escape" paths would actually be in-jail and the rejection + * assertions would invert; skip the staged-fixture checks in that + * unusual case rather than report a bogus confinement failure. */ + WMEMSET(jailCwd, 0, sizeof(jailCwd)); + escSibling[0] = '\0'; + if (WGETCWD(NULL, jailCwd, sizeof(jailCwd) - 1) != NULL) { + size_t jailLen = WSTRLEN(jailCwd); + if (WSTRLEN(escRoot) >= jailLen && + WSTRNCMP(escRoot, jailCwd, jailLen) == 0) { + WRMDIR(NULL, escRoot); + return; + } + /* "_wolfssh_confine_sibling" - a sibling of the jail sharing its + * name as a string prefix, with a distinctive test-specific suffix so + * it will not match a real user directory. The first byte past the + * jail prefix is '_' (not a delimiter), so the boundary check rejects + * it. If the name would truncate, leave escSibling empty so the + * boundary-check case is skipped rather than staged at a wrong path. */ + snLen = WSNPRINTF(escSibling, sizeof(escSibling), + "%s_wolfssh_confine_sibling", jailCwd); + if (snLen < 0 || (size_t)snLen >= sizeof(escSibling)) { + escSibling[0] = '\0'; + } + } + + WSNPRINTF(escFile, sizeof(escFile), "%s/real_file", escRoot); + WSNPRINTF(escDir, sizeof(escDir), "%s/real_dir", escRoot); + WSNPRINTF(escMkdir, sizeof(escMkdir), "%s/mkdir", escRoot); + WSNPRINTF(escDest, sizeof(escDest), "%s/renamed", escRoot); + /* climb to filesystem root with a generous ".." count (RealPath clamps the + * excess at root) then re-descend to escFile, so this relative path + * resolves to the very same out-of-jail file the absolute escFile does */ + snLen = WSNPRINTF(escRel, sizeof(escRel), + "../../../../../../../../../../../../../../../../%s", escFile + 1); + AssertIntGE(snLen, 0); + AssertIntLT(snLen, (int)sizeof(escRel)); +#ifdef WOLFSSH_HAVE_SYMLINK + /* a path that traverses the in-jail symlink out to the staged real file */ + WSNPRINTF(escSymThru, sizeof(escSymThru), "%s/real_file", escSymlink); +#endif + + /* stage the real out-of-jail file and directory */ + AssertIntEQ(WFOPEN(NULL, &fp, escFile, "wb"), 0); + AssertNotNull(fp); + WFCLOSE(NULL, fp); + AssertIntEQ(WMKDIR(NULL, escDir, 0755), 0); + + /* stage the sibling so a boundary-check regression would actually + * enumerate it (rather than fail with ENOENT). Never remove a pre-existing + * directory: only create it when absent, and if creation fails (e.g. it + * already exists, possibly user data despite the distinctive name), clear + * escSibling so the sub-test is skipped and cleanup leaves it untouched. + * escSibling is thus non-empty only when this run created the directory. */ + if (escSibling[0] != '\0') { + if (WMKDIR(NULL, escSibling, 0755) != 0) { + escSibling[0] = '\0'; + } + } + +#ifdef WOLFSSH_HAVE_SYMLINK + /* stage an in-jail symlink pointing at the out-of-jail temp root */ + WREMOVE(NULL, escSymlink); + AssertIntEQ(symlink(escRoot, escSymlink), 0); +#endif +#endif + + WMEMSET(&ser, 0, sizeof(func_args)); + argsCount = 0; + args[argsCount++] = "."; + args[argsCount++] = "-1"; +#ifndef USE_WINDOWS_API + args[argsCount++] = "-p"; + args[argsCount++] = "0"; +#endif + ser.argv = (char**)args; + ser.argc = argsCount; + ser.signal = &ready; + InitTcpReady(ser.signal); + ThreadStart(echoserver_test, (void*)&ser, &serThread); + WaitTcpReady(&ready); + + sftp_client_connect(&ctx, &ssh, ready.port); + AssertNotNull(ctx); + AssertNotNull(ssh); + + /* The client API maps PERMISSION and FAILURE both to WS_FATAL_ERROR; + * assert != WS_SUCCESS and verify the session stays alive afterward. */ + + /* Remove: out-of-jail absolute path -> rejected, session survives */ + ret = wolfSSH_SFTP_Remove(ssh, escFile); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* Remove: relative ".." traversal resolving to the same real out-of-jail + * file -> rejected by the post-RealPath containment check, session + * survives. escFile still exists afterward (a bypass would have deleted + * it, failing the absolute-path assertions on a re-run). escRel/fp are + * staged only on hosted POSIX (mkdtemp/fopen), so this case is POSIX-only; + * on Zephyr/Windows the absolute-path rejection above already covers the + * Remove sink. */ +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + ret = wolfSSH_SFTP_Remove(ssh, escRel); + AssertIntNE(ret, WS_SUCCESS); + AssertIntEQ(WFOPEN(NULL, &fp, escFile, "rb"), 0); + AssertNotNull(fp); + WFCLOSE(NULL, fp); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; +#endif + + /* RMDIR: out-of-jail path -> rejected, session survives */ + ret = wolfSSH_SFTP_RMDIR(ssh, escDir); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* MKDIR: out-of-jail path -> rejected, session survives */ + WMEMSET(&atr, 0, sizeof(atr)); + ret = wolfSSH_SFTP_MKDIR(ssh, escMkdir, &atr); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* Open: out-of-jail path -> rejected, session survives */ + handleSz = WOLFSSH_MAX_HANDLE; + ret = wolfSSH_SFTP_Open(ssh, escFile, WOLFSSH_FXF_READ, NULL, + handle, &handleSz); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* LS (OpenDir): out-of-jail path -> rejected, session survives */ + ls = wolfSSH_SFTP_LS(ssh, escDir); + AssertNull(ls); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + /* LS (OpenDir) on the "_wolfssh_confine_sibling" sibling: its resolved + * path shares the jail prefix exactly but the next byte is '_' (not a + * delimiter), so it must be rejected by the boundary check even though a + * plain prefix compare would accept it. The dir really exists, so a + * regression would return a non-NULL listing. */ + if (escSibling[0] != '\0') { + ls = wolfSSH_SFTP_LS(ssh, escSibling); + AssertNull(ls); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + } +#endif + + /* Rename: out-of-jail path -> rejected, session survives */ + ret = wolfSSH_SFTP_Rename(ssh, escFile, escDest); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* STAT: out-of-jail path -> rejected, session survives */ + WMEMSET(&atr, 0, sizeof(atr)); + ret = wolfSSH_SFTP_STAT(ssh, escFile, &atr); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* LSTAT: out-of-jail path -> rejected, session survives */ + WMEMSET(&atr, 0, sizeof(atr)); + ret = wolfSSH_SFTP_LSTAT(ssh, escFile, &atr); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* SetSTAT: out-of-jail path -> rejected, session survives */ + WMEMSET(&atr, 0, sizeof(atr)); + ret = wolfSSH_SFTP_SetSTAT(ssh, escFile, &atr); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) && \ + defined(WOLFSSH_HAVE_SYMLINK) + /* Symlink escape: an in-jail symlink to the out-of-jail tree resolves to + * an in-jail path string, so the prefix check alone would pass; the + * per-component link check must reject both listing the link itself and + * opening a file through it. Without the fix these would follow the link + * and succeed, escaping the jail. Guarded to match the POSIX-only staging + * above (mkdtemp/symlink) and WOLFSSH_HAVE_SYMLINK so it only runs where + * both the fixtures and the server-side link check exist. */ + ls = wolfSSH_SFTP_LS(ssh, escSymlink); + AssertNull(ls); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + handleSz = WOLFSSH_MAX_HANDLE; + ret = wolfSSH_SFTP_Open(ssh, escSymThru, WOLFSSH_FXF_READ, NULL, + handle, &handleSz); + AssertIntNE(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; +#endif + + /* Positive case: a relative write op that resolves inside the jail must be + * allowed. This guards the GetAndCleanPath prefix-compare allow path (and + * the s[dpLen] boundary check) against an over-restrictive regression that + * would only be caught by the broader CI shell scripts otherwise. MKDIR + * the in-jail name, assert success, then RMDIR it back to a clean state. */ + WMEMSET(&atr, 0, sizeof(atr)); + ret = wolfSSH_SFTP_MKDIR(ssh, inJailDir, &atr); + AssertIntEQ(ret, WS_SUCCESS); + ret = wolfSSH_SFTP_RMDIR(ssh, inJailDir); + AssertIntEQ(ret, WS_SUCCESS); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + + /* Drain any pending rekey before shutdown. */ + while (wolfSSH_get_error(ssh) == WS_REKEYING) + wolfSSH_worker(ssh, NULL); + + ret = wolfSSH_shutdown(ssh); + if (ret == WS_SOCKET_ERROR_E) { + ret = WS_SUCCESS; + } +#if DEFAULT_HIGHWATER_MARK < 8000 + if (ret == WS_REKEYING) { + ret = WS_SUCCESS; + } +#endif + AssertIntEQ(ret, WS_SUCCESS); + clientFd = wolfSSH_get_fd(ssh); + WCLOSESOCKET(clientFd); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +#ifdef WOLFSSH_ZEPHYR + k_sleep(Z_TIMEOUT_TICKS(100)); +#endif + ThreadJoin(serThread); + + /* remove staged targets; escMkdir/escDest only exist if confinement + * leaked, and inJailDir only if the positive-case RMDIR did not run, so + * their removal is best effort */ + WREMOVE(NULL, escFile); + WRMDIR(NULL, escDir); + WRMDIR(NULL, escMkdir); + WREMOVE(NULL, escDest); + WRMDIR(NULL, inJailDir); +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) +#ifdef WOLFSSH_HAVE_SYMLINK + WREMOVE(NULL, escSymlink); +#endif + WRMDIR(NULL, escRoot); + /* escSibling is non-empty only if this run created it (see staging above), + * so this never removes a pre-existing directory belonging to the user */ + if (escSibling[0] != '\0') { + WRMDIR(NULL, escSibling); + } +#endif +} + + +/* Direct unit coverage for wolfSSH_SFTP_SetDefaultPath, exercising the new + * canonicalization and error branches that test_wolfSSH_SFTP_Confinement only + * reaches indirectly (it always passes an already-absolute realpath): + * NULL ssh, the too-long-path guard, NULL path (no change), absolute-path + * canonicalization, the repeated-call free path, and relative-path resolution + * against the canonicalized cwd. */ +static void test_wolfSSH_SFTP_SetDefaultPath(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + char longPath[WOLFSSH_MAX_FILENAME + 4]; +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + char cwdBuf[WOLFSSH_MAX_FILENAME]; + char cwdReal[WOLFSSH_MAX_FILENAME]; + char expect[WOLFSSH_MAX_FILENAME]; + char rel[] = "sdp_rel_seg"; +#endif + + AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL)); + AssertNotNull(ssh = wolfSSH_new(ctx)); + + /* NULL ssh is rejected */ + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(NULL, "/"), WS_BAD_ARGUMENT); + + /* NULL path leaves the (still unset) default path unchanged */ + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, NULL), WS_SUCCESS); + AssertNull(ssh->sftpDefaultPath); + + /* A path that does not fit the working buffer is rejected up front and + * does not store anything */ + WMEMSET(longPath, 'a', sizeof(longPath)); + longPath[0] = '/'; + longPath[WOLFSSH_MAX_FILENAME + 1] = '\0'; /* length == MAX_FILENAME + 1 */ + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, longPath), WS_BUFFER_E); + AssertNull(ssh->sftpDefaultPath); + + /* An absolute path is stored in lexically canonical form */ + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, "/tmp/../tmp/sdp"), + WS_SUCCESS); + AssertNotNull(ssh->sftpDefaultPath); + AssertStrEQ(ssh->sftpDefaultPath, "/tmp/sdp"); + + /* A repeated call frees the previous path (no leak) and stores the new + * one - the wolfsshd "/" then home-dir sequence */ + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, "/var/sdp2"), WS_SUCCESS); + AssertNotNull(ssh->sftpDefaultPath); + AssertStrEQ(ssh->sftpDefaultPath, "/var/sdp2"); + +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + /* A relative path is resolved against the canonicalized cwd, so the stored + * path is absolute and matches cwd + "/seg" rather than a lexical "/seg" - + * confirming the relative branch ran. The expected value is built with + * the same two RealPath passes the implementation uses. */ + AssertNotNull(WGETCWD(NULL, cwdBuf, sizeof(cwdBuf) - 1)); + AssertIntEQ(wolfSSH_RealPath(NULL, cwdBuf, cwdReal, sizeof(cwdReal)), + WS_SUCCESS); + AssertIntEQ(wolfSSH_RealPath(cwdReal, rel, expect, sizeof(expect)), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, "sdp_rel_seg"), WS_SUCCESS); + AssertNotNull(ssh->sftpDefaultPath); + AssertStrEQ(ssh->sftpDefaultPath, expect); +#endif + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + #else /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */ static void test_wolfSSH_SFTP_SendReadPacket(void) { ; } static void test_wolfSSH_SFTP_ReKey(void) { ; } static void test_wolfSSH_SFTP_ReKey_NonBlock(void) { ; } +static void test_wolfSSH_SFTP_Confinement(void) { ; } +static void test_wolfSSH_SFTP_SetDefaultPath(void) { ; } #endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */ @@ -1790,6 +2250,7 @@ static void DoRealPathTestFailCase(struct RealPathTestFailCase* tc) } + static void test_wolfSSH_RealPath(void) { word32 testCount; @@ -2299,6 +2760,8 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_SFTP_SendReadPacket(); test_wolfSSH_SFTP_ReKey(); test_wolfSSH_SFTP_ReKey_NonBlock(); + test_wolfSSH_SFTP_Confinement(); + test_wolfSSH_SFTP_SetDefaultPath(); /* Either SCP or SFTP */ test_wolfSSH_RealPath(); diff --git a/wolfssh/port.h b/wolfssh/port.h index fee7ab39b..984accaf0 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1505,6 +1505,22 @@ extern "C" { #endif #endif /* WOLFSSH_SFTP or WOLFSSH_SCP */ +/* SFTP path confinement must reject symbolic links because wolfSSH_RealPath + * only normalizes the path string and does not resolve them. Define + * WOLFSSH_HAVE_SYMLINK on the filesystems that can actually store a link - + * POSIX (checked with lstat) and Windows (reparse points) - so the check is + * compiled out on FAT-based and similar link-less embedded filesystems, and on + * user-supplied filesystems that own their own policy. Define + * WOLFSSH_NO_SYMLINK_CHECK to force it off. */ +#if !defined(WOLFSSH_NO_SYMLINK_CHECK) && \ + (defined(USE_WINDOWS_API) || \ + (!defined(WOLFSSL_NUCLEUS) && !defined(FREESCALE_MQX) && \ + !defined(WOLFSSH_FATFS) && !defined(WOLFSSH_ZEPHYR) && \ + !defined(MICROCHIP_MPLAB_HARMONY) && !defined(WOLFSSH_USER_FILESYSTEM) && \ + !defined(USE_OSE_API) && !defined(NO_FILESYSTEM))) + #define WOLFSSH_HAVE_SYMLINK +#endif + #ifndef WS_MAYBE_UNUSED #if (defined(__GNUC__) && (__GNUC__ >= 4)) || defined(__clang__) || \ defined(__IAR_SYSTEMS_ICC__)