From 4cad402ea8a91031f86c53961d78bb7f4f174790 Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Sun, 13 Apr 2014 10:36:59 -0700 Subject: Receiver now rejects invalid filenames in filelist. 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 | 14 ++++++++------ rsync.h | 1 + util.c | 14 ++++++++++---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/flist.c b/flist.c index a0f05dd0..74c07564 100644 --- 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 - 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); @@ -2554,10 +2557,9 @@ struct file_list *recv_file_list(int f) } /* 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 */ diff --git a/rsync.h b/rsync.h index 205029b7..4fef8827 100644 --- a/rsync.h +++ b/rsync.h @@ -208,6 +208,7 @@ #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) diff --git a/util.c b/util.c index efd3c327..bd537ae9 100644 --- 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 ".". */ -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; @@ -880,6 +880,8 @@ unsigned int clean_fname(char *name, int flags) 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__ @@ -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_REFUSE_DOT_DOT_DIRS && *f == '.' && DOT_IS_DOT_DOT_DIR(f)) + return -1; 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 (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; + if (flags & CFN_REFUSE_DOT_DOT_DIRS) + return -1; if (s == name && anchored) { f += 2; continue; @@ -933,6 +937,8 @@ unsigned int clean_fname(char *name, int flags) *t++ = '.'; *t = '\0'; +#undef DOT_IS_DOT_DOT_DIR + return t - name; } -- cgit v1.2.1