Receiver now rejects invalid filenames in filelist.
authorWayne Davison <wayned@samba.org>
Sun, 13 Apr 2014 17:36:59 +0000 (10:36 -0700)
committerWayne Davison <wayned@samba.org>
Sun, 13 Apr 2014 17:36:59 +0000 (10:36 -0700)
If the receiver gets a filename with a leading slash (w/o --relative)
and/or a filename with an embedded ".." dir in the path, it dies with
an error (rather than continuing). Those invalid paths should never
happen in reality, so just reject someone trying to pull a fast one.

flist.c
rsync.h
util.c

diff --git a/flist.c b/flist.c
index a0f05dd020a75d323e1e91a18142dd4e08388267..74c0756482459572beb70311d68c803a0cd5ab01 100644 (file)
--- a/flist.c
+++ b/flist.c
@@ -736,8 +736,11 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
        }
 #endif
 
        }
 #endif
 
-       if (*thisname)
-               clean_fname(thisname, 0);
+       if (*thisname
+        && (clean_fname(thisname, CFN_REFUSE_DOT_DOT_DIRS) < 0 || (!relative_paths && *thisname == '/'))) {
+               rprintf(FERROR, "ABORTING due to unsafe pathname from sender: %s\n", thisname);
+               exit_cleanup(RERR_PROTOCOL);
+       }
 
        if (sanitize_paths)
                sanitize_path(thisname, thisname, "", 0, SP_DEFAULT);
 
        if (sanitize_paths)
                sanitize_path(thisname, thisname, "", 0, SP_DEFAULT);
@@ -2554,10 +2557,9 @@ struct file_list *recv_file_list(int f)
        }
 
        /* The --relative option sends paths with a leading slash, so we need
        }
 
        /* The --relative option sends paths with a leading slash, so we need
-        * to specify the strip_root option here.  We also want to ensure that
-        * a non-relative transfer doesn't have any leading slashes or it might
-        * cause the client a security issue. */
-       flist_sort_and_clean(flist, 1);
+        * to specify the strip_root option here.  We rejected leading slashes
+        * for a non-relative transfer in recv_file_entry(). */
+       flist_sort_and_clean(flist, relative_paths);
 
        if (protocol_version < 30) {
                /* Recv the io_error flag */
 
        if (protocol_version < 30) {
                /* Recv the io_error flag */
diff --git a/rsync.h b/rsync.h
index 205029b7278d1770ea21614c2e7f996d9fe15439..4fef8827ac5578b92ab9f9fa761aaa52200f9c6a 100644 (file)
--- a/rsync.h
+++ b/rsync.h
 #define CFN_KEEP_TRAILING_SLASH (1<<1)
 #define CFN_DROP_TRAILING_DOT_DIR (1<<2)
 #define CFN_COLLAPSE_DOT_DOT_DIRS (1<<3)
 #define CFN_KEEP_TRAILING_SLASH (1<<1)
 #define CFN_DROP_TRAILING_DOT_DIR (1<<2)
 #define CFN_COLLAPSE_DOT_DOT_DIRS (1<<3)
+#define CFN_REFUSE_DOT_DOT_DIRS (1<<4)
 
 #define SP_DEFAULT 0
 #define SP_KEEP_DOT_DIRS (1<<0)
 
 #define SP_DEFAULT 0
 #define SP_KEEP_DOT_DIRS (1<<0)
diff --git a/util.c b/util.c
index efd3c3276de8cfce826db12dab8cf14211403ca3..bd537ae9bfa6d2151422a42ee72520b99da294b9 100644 (file)
--- a/util.c
+++ b/util.c
@@ -872,7 +872,7 @@ int count_dir_elements(const char *p)
  * CFN_KEEP_TRAILING_SLASH is flagged, and will also collapse ".." elements
  * (except at the start) if CFN_COLLAPSE_DOT_DOT_DIRS is flagged.  If the
  * resulting name would be empty, returns ".". */
  * CFN_KEEP_TRAILING_SLASH is flagged, and will also collapse ".." elements
  * (except at the start) if CFN_COLLAPSE_DOT_DOT_DIRS is flagged.  If the
  * resulting name would be empty, returns ".". */
-unsigned int clean_fname(char *name, int flags)
+int clean_fname(char *name, int flags)
 {
        char *limit = name - 1, *t = name, *f = name;
        int anchored;
 {
        char *limit = name - 1, *t = name, *f = name;
        int anchored;
@@ -880,6 +880,8 @@ unsigned int clean_fname(char *name, int flags)
        if (!name)
                return 0;
 
        if (!name)
                return 0;
 
+#define DOT_IS_DOT_DOT_DIR(bp) (bp[1] == '.' && (bp[2] == '/' || !bp[2]))
+
        if ((anchored = *f == '/') != 0) {
                *t++ = *f++;
 #ifdef __CYGWIN__
        if ((anchored = *f == '/') != 0) {
                *t++ = *f++;
 #ifdef __CYGWIN__
@@ -892,7 +894,8 @@ unsigned int clean_fname(char *name, int flags)
        } else if (flags & CFN_KEEP_DOT_DIRS && *f == '.' && f[1] == '/') {
                *t++ = *f++;
                *t++ = *f++;
        } else if (flags & CFN_KEEP_DOT_DIRS && *f == '.' && f[1] == '/') {
                *t++ = *f++;
                *t++ = *f++;
-       }
+       } else if (flags & CFN_REFUSE_DOT_DOT_DIRS && *f == '.' && DOT_IS_DOT_DOT_DIR(f))
+               return -1;
        while (*f) {
                /* discard extra slashes */
                if (*f == '/') {
        while (*f) {
                /* discard extra slashes */
                if (*f == '/') {
@@ -908,9 +911,10 @@ unsigned int clean_fname(char *name, int flags)
                        if (f[1] == '\0' && flags & CFN_DROP_TRAILING_DOT_DIR)
                                break;
                        /* collapse ".." dirs */
                        if (f[1] == '\0' && flags & CFN_DROP_TRAILING_DOT_DIR)
                                break;
                        /* collapse ".." dirs */
-                       if (flags & CFN_COLLAPSE_DOT_DOT_DIRS
-                        && f[1] == '.' && (f[2] == '/' || !f[2])) {
+                       if (flags & (CFN_COLLAPSE_DOT_DOT_DIRS|CFN_REFUSE_DOT_DOT_DIRS) && DOT_IS_DOT_DOT_DIR(f)) {
                                char *s = t - 1;
                                char *s = t - 1;
+                               if (flags & CFN_REFUSE_DOT_DOT_DIRS)
+                                       return -1;
                                if (s == name && anchored) {
                                        f += 2;
                                        continue;
                                if (s == name && anchored) {
                                        f += 2;
                                        continue;
@@ -933,6 +937,8 @@ unsigned int clean_fname(char *name, int flags)
                *t++ = '.';
        *t = '\0';
 
                *t++ = '.';
        *t = '\0';
 
+#undef DOT_IS_DOT_DOT_DIR
+
        return t - name;
 }
 
        return t - name;
 }