General mechanism for ensuring a dodgy SFTP server can't return

malicious filenames via FXP_READDIR.

git-svn-id: svn://svn.tartarus.org/sgt/putty@4995 cda61777-01e9-0310-a592-d414129be87e

diff --git a/pscp.c b/pscp.c
index 1c601e9..0fa1839 100644 (file)
--- a/pscp.c
+++ b/pscp.c
@@ -687,7 +687,6 @@ void scp_sftp_listdir(char *dirname)
 
            for (i = 0; i < names->nnames; i++)
                ournames[nnames++] = names->names[i];
-
            names->nnames = 0;         /* prevent free_names */
            fxp_free_names(names);
        }
@@ -1289,8 +1288,21 @@ int scp_get_sink_action(struct scp_sink_action *act)
                    namesize += names->nnames + 128;
                    ournames = sresize(ournames, namesize, struct fxp_name);
                }
-               for (i = 0; i < names->nnames; i++)
-                   ournames[nnames++] = names->names[i];
+               for (i = 0; i < names->nnames; i++) {
+                   if (!strcmp(names->names[i].filename, ".") ||
+                       !strcmp(names->names[i].filename, "..")) {
+                       /*
+                        * . and .. are normal consequences of
+                        * reading a directory, and aren't worth
+                        * complaining about.
+                        */
+                   } else if (!vet_filename(names->names[i].filename)) {
+                       tell_user(stderr, "ignoring potentially dangerous server-"
+                                 "supplied filename '%s'\n",
+                                 names->names[i].filename);
+                   } else
+                       ournames[nnames++] = names->names[i];
+               }
                names->nnames = 0;             /* prevent free_names */
                fxp_free_names(names);
            }

diff --git a/psftp.c b/psftp.c
index b1cfe16..1612b3b 100644 (file)
--- a/psftp.c
+++ b/psftp.c
@@ -41,14 +41,6 @@ static Config cfg;
  */
 
 /*
- * Determine whether a string is entirely composed of dots.
- */
-static int is_dots(char *str)
-{
-    return str[strspn(str, ".")] == '\0';
-}
-
-/*
  * Attempt to canonify a pathname starting from the pwd. If
  * canonification fails, at least fall back to returning a _valid_
  * pathname (though it may be ugly, eg /home/simon/../foobar).
@@ -291,10 +283,19 @@ int sftp_get_file(char *fname, char *outfname, int recurse, int restart,
                    ournames = sresize(ournames, namesize, struct fxp_name *);
                }
                for (i = 0; i < names->nnames; i++)
-                   if (!is_dots(names->names[i].filename) &&
+                   if (strcmp(names->names[i].filename, ".") &&
+                       strcmp(names->names[i].filename, "..") &&
                        (!wildcard || wc_match(wildcard,
-                                              names->names[i].filename)))
-                       ournames[nnames++] = fxp_dup_name(&names->names[i]);
+                                              names->names[i].filename))) {
+                       if (!vet_filename(names->names[i].filename)) {
+                           printf("ignoring potentially dangerous server-"
+                                  "supplied filename '%s'\n",
+                                  names->names[i].filename);
+                       } else {
+                           ournames[nnames++] =
+                               fxp_dup_name(&names->names[i]);
+                       }
+                   }
                fxp_free_names(names);
            }
            sftp_register(req = fxp_close_send(dirhandle));

diff --git a/psftp.h b/psftp.h
index af8917d..2f323c7 100644 (file)
--- a/psftp.h
+++ b/psftp.h
@@ -150,6 +150,16 @@ char *wildcard_get_filename(WildcardMatcher *dir);
 void finish_wildcard_matching(WildcardMatcher *dir);
 
 /*
+ * Vet a filename returned from the remote host, to ensure it isn't
+ * in some way malicious. The idea is that this function is applied
+ * to filenames returned from FXP_READDIR, which means we can panic
+ * if we see _anything_ resembling a directory separator.
+ * 
+ * Returns TRUE if the filename is kosher, FALSE if dangerous.
+ */
+int vet_filename(char *name);
+
+/*
  * Create a directory. Returns 0 on error, !=0 on success.
  */
 int create_directory(char *name);

diff --git a/unix/uxsftp.c b/unix/uxsftp.c
index 9958a79..7045c32 100644 (file)
--- a/unix/uxsftp.c
+++ b/unix/uxsftp.c
@@ -341,6 +341,17 @@ void finish_wildcard_matching(WildcardMatcher *dir) {
     sfree(dir);
 }
 
+int vet_filename(char *name)
+{
+    if (strchr(name, '/'))
+       return FALSE;
+
+    if (name[0] == '.' && (!name[1] || (name[1] == '.' && !name[2])))
+       return FALSE;
+
+    return TRUE;
+}
+
 int create_directory(char *name)
 {
     return mkdir(name, 0777) == 0;

diff --git a/windows/winsftp.c b/windows/winsftp.c
index c071257..82bea80 100644 (file)
--- a/windows/winsftp.c
+++ b/windows/winsftp.c
@@ -445,6 +445,17 @@ void finish_wildcard_matching(WildcardMatcher *dir)
     sfree(dir);
 }
 
+int vet_filename(char *name)
+{
+    if (strchr(name, '/') || strchr(name, '\\') || strchr(name, ':'))
+       return FALSE;
+
+    if (!name[strspn(name, ".")])      /* entirely composed of dots */
+       return FALSE;
+
+    return TRUE;
+}
+
 int create_directory(char *name)
 {
     return CreateDirectory(name, NULL) != 0;