Merge pull request #743 from JacobBarthelmeh/sftp

keep trailing delimiter with SFTP open

Author: douzzer

apps/wolfsshd/test/run_all_sshd_tests.sh

@@ -60,6 +60,7 @@ run_test() {
 run_test "sshd_exec_test.sh"
 run_test "sshd_term_size_test.sh"
 run_test "sshd_large_sftp_test.sh"
+run_test "sshd_bad_sftp_test.sh"
 
 #Github actions needs resolved for these test cases
 #run_test "error_return.sh"

apps/wolfsshd/test/sshd_bad_sftp_test.sh

@@ -0,0 +1,34 @@
+#!/bin/sh
+
+# sshd local test
+
+PWD=`pwd`
+cd ../../..
+
+TEST_SFTP_CLIENT="./examples/sftpclient/wolfsftp"
+USER=`whoami`
+PRIVATE_KEY="./keys/hansel-key-ecc.der"
+PUBLIC_KEY="./keys/hansel-key-ecc.pub"
+
+if [ -z "$1" ] || [ -z "$2" ]; then
+    echo "expecting host and port as arguments"
+    echo "./sshd_exec_test.sh 127.0.0.1 22222"
+    exit 1
+fi
+
+mkdir test-$$
+mkdir test-$$/subfolder
+
+echo "$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l configure -r `pwd`/test-$$/subfolder/ -h \"$1\" -p \"$2\""
+"$TEST_SFTP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -g -l configure -r `pwd`/test-$$/subfolder/ -h $1 -p $2"
+
+RESULT=$?
+if [ "$RESULT" = "0" ]; then
+    echo "Expecting to fail transfer to folder"
+    exit 1
+fi
+rm -rf test-$$
+
+cd $PWD
+exit 0
+

src/internal.c

@@ -15691,6 +15691,66 @@ int SendChannelSuccess(WOLFSSH* ssh, word32 channelId, int success)
 
 #if (defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP)) && \
     !defined(NO_WOLFSSH_SERVER)
+/* Checks if 'in' is absolute path, if not the returns the concat. of
+ * 'defaultPath' | 'in'. This leaves 'in' as-is and does not handle
+ * simplification of the path, such as removing ../
+ *
+ * Sanity checks outSz and then adjusts it for the size used
+ * returns WS_SUCCESS on success
+ */
+int wolfSSH_GetPath(const char* defaultPath, byte* in, word32 inSz,
+        char* out, word32* outSz)
+{
+    word32 curSz = 0;
+
+    if (out != NULL) {
+        WMEMSET(out, 0, *outSz);
+    }
+
+    if (inSz == 0 || (!WOLFSSH_SFTP_IS_DELIM(in[0]) &&
+        !WOLFSSH_SFTP_IS_WINPATH(inSz, in))) {
+        if (defaultPath != NULL) {
+            curSz = (word32)WSTRLEN(defaultPath);
+            if (out != NULL && curSz >= *outSz) {
+                return WS_INVALID_PATH_E;
+            }
+            if (out != NULL) {
+                WSTRNCPY(out, defaultPath, *outSz);
+                if (out[curSz] != '/') {
+                    out[curSz] = '/';
+                    curSz++;
+                }
+            }
+        }
+    }
+
+    if (out != NULL) {
+        if (curSz + inSz < *outSz) {
+            WMEMCPY(out + curSz, in, inSz);
+        }
+    }
+    curSz += inSz;
+    if (out != NULL) {
+        if (!WOLFSSH_SFTP_IS_DELIM(out[0])) {
+            if (curSz + 1 < *outSz) {
+                WMEMMOVE(out+1, out, curSz);
+                out[0] = '/';
+            }
+            curSz++;
+        }
+        if (curSz < *outSz) {
+            out[curSz] = 0;
+        }
+        else {
+            return WS_BUFFER_E;
+        }
+    }
+    *outSz = curSz;
+
+    return WS_SUCCESS;
+}
+
+
 /* cleans up absolute path
  * returns size of new path on success (strlen sz) and negative values on fail*/
 int wolfSSH_CleanPath(WOLFSSH* ssh, char* in)

src/ssh.c

@@ -3276,10 +3276,6 @@ void* wolfSSH_GetChannelCloseCtx(WOLFSSH* ssh)
 #if (defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP)) && \
     !defined(NO_WOLFSSH_SERVER)
 
-#define DELIM "/\\"
-#define IS_DELIM(x) ((x) == '/' || (x) == '\\')
-#define IS_WINPATH(x,y) ((x) > 1 && (y)[1] == ':')
-
 /*
  * Paths starting with a slash are absolute, rooted at "/". Any path that
  * doesn't have a starting slash is assumed to be relative to the default
@@ -3288,6 +3284,8 @@ void* wolfSSH_GetChannelCloseCtx(WOLFSSH* ssh)
  * The path "/." is stripped out. The path "/.." strips out the previous
  * path value. The root path, "/", is always present.
  *
+ * Trailing delimiters are stripped, i.e /tmp/path/ becomes /tmp/path
+ *
  * Example: "/home/fred/frob/frizz/../../../barney/bar/baz/./././../.."
  * will return "/home/barney". "/../.." will return "/". "." will return
  * the default path.
@@ -3319,7 +3317,8 @@ int wolfSSH_RealPath(const char* defaultPath, char* in,
     inSz = (word32)WSTRLEN(in);
     out[0] = '/';
     curSz = 1;
-    if (inSz == 0 || (!IS_DELIM(in[0]) && !IS_WINPATH(inSz, in))) {
+    if (inSz == 0 || (!WOLFSSH_SFTP_IS_DELIM(in[0]) &&
+        !WOLFSSH_SFTP_IS_WINPATH(inSz, in))) {
         if (defaultPath != NULL) {
             curSz = (word32)WSTRLEN(defaultPath);
             if (curSz >= outSz) {
@@ -3330,9 +3329,9 @@ int wolfSSH_RealPath(const char* defaultPath, char* in,
     }
     out[curSz] = 0;
 
-    for (seg = WSTRTOK(in, DELIM, &tail);
+    for (seg = WSTRTOK(in, WOLFSSH_SFTP_DELIM, &tail);
             seg;
-            seg = WSTRTOK(NULL, DELIM, &tail)) {
+            seg = WSTRTOK(NULL, WOLFSSH_SFTP_DELIM, &tail)) {
         segSz = (word32)WSTRLEN(seg);
 
         /* Try to match "." */

src/wolfsftp.c

@@ -1974,7 +1974,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
 {
     WS_SFTP_FILEATRB atr;
     WFD    fd;
-    word32 sz;
+    word32 sz, dirSz;
     char   dir[WOLFSSH_MAX_FILENAME];
     word32 reason;
     word32 idx = 0;
@@ -2010,10 +2010,11 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
         return WS_BUFFER_E;
     }
 
-    ret = GetAndCleanPath(ssh->sftpDefaultPath, data + idx, sz,
-            dir, sizeof(dir));
-    if (ret < 0) {
-        return ret;
+    dirSz = sizeof(dir);
+    if (wolfSSH_GetPath(ssh->sftpDefaultPath, data + idx, sz, dir, &dirSz)
+            != WS_SUCCESS) {
+        WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
+        return WS_FATAL_ERROR;
     }
     idx += sz;
 
@@ -2128,7 +2129,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
 {
 /*    WS_SFTP_FILEATRB atr;*/
     HANDLE fileHandle;
-    word32 sz;
+    word32 sz, dirSz;
     char   dir[WOLFSSH_MAX_FILENAME];
     word32 reason;
     word32 idx = 0;
@@ -2165,9 +2166,11 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
         return WS_BUFFER_E;
     }
 
-    if (GetAndCleanPath(ssh->sftpDefaultPath,
-                data + idx, sz, dir, sizeof(dir)) < 0) {
-        return WS_BUFFER_E;
+    dirSz = sizeof(dir);
+    if (wolfSSH_GetPath(ssh->sftpDefaultPath, data + idx, sz, dir, &dirSz)
+            != WS_SUCCESS) {
+        WLOG(WS_LOG_SFTP, "Creating path for file to open failed");
+        return WS_FATAL_ERROR;
     }
     idx += sz;
 

wolfssh/internal.h

@@ -620,6 +620,12 @@ typedef struct HandshakeInfo {
     } privKey;
 } HandshakeInfo;
 
+#if (defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP)) && \
+    !defined(NO_WOLFSSH_SERVER)
+WOLFSSH_LOCAL int wolfSSH_GetPath(const char* defaultPath, byte* in,
+    word32 inSz, char* out, word32* outSz);
+#endif
+
 #ifdef WOLFSSH_SFTP
 #define WOLFSSH_MAX_SFTPOFST 3
 

wolfssh/port.h

@@ -575,6 +575,14 @@ extern "C" {
     #define WLOCALTIME(c,r) (localtime_r((c),(r))!=NULL)
 #endif
 
+#ifndef WOLFSSH_SFTP_DELIM
+    /* Delimiter's used between two SFTP peers should be the same regardless of
+     * operating system. WS_DELIM defined elsewhere is OS specific delimiter. */
+    #define WOLFSSH_SFTP_DELIM "/\\"
+    #define WOLFSSH_SFTP_IS_DELIM(x) ((x) == '/' || (x) == '\\')
+    #define WOLFSSH_SFTP_IS_WINPATH(x,y) ((x) > 1 && (y)[1] == ':')
+#endif
+
 #if (defined(WOLFSSH_SFTP) || \
         defined(WOLFSSH_SCP) || defined(WOLFSSH_SSHD)) && \
     !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM)