From 2aee4549a3e64458fc232878c51cb5cf8f26d494 Mon Sep 17 00:00:00 2001 From: Matt McCutchen Date: Mon, 5 Oct 2009 23:56:00 -0400 Subject: [PATCH] Attempt to avoid a problem with --copy-unsafe-links where a symlink was considered safe even though it walked out of the tree established by a previous unsafe symlink. http://lists.samba.org/archive/rsync/2009-October/024003.html --- backup.c | 6 ++--- flist.c | 68 ++++++++++++++++++++++++++++++++++++---------------- generator.c | 6 ++--- rsync.h | 8 +++++++ t_unsafe.c | 2 +- util.c | 69 +++++++++++++++++++++++++++++++---------------------- 6 files changed, 104 insertions(+), 55 deletions(-) diff --git a/backup.c b/backup.c index 8512e00c..840bab7f 100644 --- a/backup.c +++ b/backup.c @@ -98,7 +98,7 @@ static BOOL copy_valid_path(const char *fname) rsyserr(FERROR, errno, "backup stat %s failed", full_fname(rel)); else { struct file_struct *file; - if (!(file = make_file(rel, NULL, NULL, 0, NO_FILTERS))) + if (!(file = make_file(rel, NULL, NULL, 0, NO_FILTERS, PHYS_DEPTH_N_A))) continue; #ifdef SUPPORT_ACLS if (preserve_acls && !S_ISLNK(file->mode)) { @@ -221,7 +221,7 @@ int make_backup(const char *fname, BOOL prefer_rename) } /* Fall back to making a copy. */ - if (!(file = make_file(fname, NULL, &sx.st, 0, NO_FILTERS))) + if (!(file = make_file(fname, NULL, &sx.st, 0, NO_FILTERS, PHYS_DEPTH_N_A))) return 1; /* the file could have disappeared */ #ifdef SUPPORT_ACLS @@ -252,7 +252,7 @@ int make_backup(const char *fname, BOOL prefer_rename) #ifdef SUPPORT_LINKS if (!ret && preserve_links && S_ISLNK(file->mode)) { const char *sl = F_SYMLINK(file); - if (safe_symlinks && unsafe_symlink(sl, fname)) { + if (safe_symlinks && unsafe_symlink(sl, path_depth(fname))) { if (INFO_GTE(SYMSAFE, 1)) { rprintf(FINFO, "ignoring unsafe symlink %s -> %s\n", full_fname(buf), sl); diff --git a/flist.c b/flist.c index a167c3b9..1819e39c 100644 --- a/flist.c +++ b/flist.c @@ -198,7 +198,8 @@ void show_flist_stats(void) * * The stat structure pointed to by stp will contain information about the * link or the referent as appropriate, if they exist. */ -static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf) +static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf, + unsigned int *phys_depth_p) { #ifdef SUPPORT_LINKS if (link_stat(path, stp, copy_dirlinks) < 0) @@ -208,11 +209,12 @@ static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf) if (llen < 0) return -1; linkbuf[llen] = '\0'; - if (copy_unsafe_links && unsafe_symlink(linkbuf, path)) { + if (copy_unsafe_links && unsafe_symlink(linkbuf, *phys_depth_p)) { if (INFO_GTE(SYMSAFE, 1)) { rprintf(FINFO,"copying unsafe symlink \"%s\" -> \"%s\"\n", path, linkbuf); } + *phys_depth_p = 0; return x_stat(path, stp, NULL); } if (munge_symlinks && am_sender && llen > SYMLINK_PREFIX_LEN @@ -221,6 +223,8 @@ static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf) llen - SYMLINK_PREFIX_LEN + 1); } } + if (copy_unsafe_links && S_ISDIR(stp->st_mode)) + (*phys_depth_p)++; return 0; #else return x_stat(path, stp, NULL); @@ -301,7 +305,7 @@ static int is_excluded(const char *fname, int is_dir, int filter_level) } static void send_directory(int f, struct file_list *flist, - char *fbuf, int len, int flags); + char *fbuf, int len, int flags, unsigned int phys_depth); static const char *pathname, *orig_dir; static int pathname_len; @@ -1131,6 +1135,12 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x /* Create a file_struct for a named file by reading its stat() information * and performing extensive checks against global options. * + * If we're the sender and --copy-unsafe-links is on, we want to consider a + * symlink unsafe if it leaves the subtree established by another unsafe + * symlink, but that can't be determined based on "fname". So the depth of + * real directories since the last unsafe symlink is passed as "phys_depth". + * In other cases, "phys_depth" is ignored. + * * Returns a pointer to the new file struct, or NULL if there was an error * or this file should be excluded. * @@ -1139,7 +1149,8 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x * "io_error |= IOERR_GENERAL" to avoid deletion of the file from the * destination if --delete is on. */ struct file_struct *make_file(const char *fname, struct file_list *flist, - STRUCT_STAT *stp, int flags, int filter_level) + STRUCT_STAT *stp, int flags, int filter_level, + unsigned int phys_depth) { static char *lastdir; static int lastdir_len = -1; @@ -1167,7 +1178,7 @@ struct file_struct *make_file(const char *fname, struct file_list *flist, * dir, or a request to delete a specific file. */ st = *stp; *linkname = '\0'; /* make IBM code checker happy */ - } else if (readlink_stat(thisname, &st, linkname) != 0) { + } else if (readlink_stat(thisname, &st, linkname, &phys_depth) != 0) { int save_errno = errno; /* See if file is excluded before reporting an error. */ if (filter_level != NO_FILTERS @@ -1314,6 +1325,9 @@ struct file_struct *make_file(const char *fname, struct file_list *flist, extra_len += SUM_EXTRA_CNT * EXTRA_LEN; } + if (copy_unsafe_links && S_ISDIR(st.st_mode)) + extra_len += EXTRA_LEN; // F_DIR_PHYS_DEPTH + #if EXTRA_ROUNDING > 0 if (extra_len & (EXTRA_ROUNDING * EXTRA_LEN)) extra_len = (extra_len | (EXTRA_ROUNDING * EXTRA_LEN)) + EXTRA_LEN; @@ -1398,6 +1412,9 @@ struct file_struct *make_file(const char *fname, struct file_list *flist, if (sender_keeps_checksum && S_ISREG(st.st_mode)) memcpy(F_SUM(file), tmp_sum, checksum_len); + if (copy_unsafe_links && S_ISDIR(st.st_mode)) + F_DIR_PHYS_DEPTH(file) = phys_depth; /* as adjusted by readlink_stat */ + if (unsort_ndx) F_NDX(file) = stats.num_dirs; @@ -1412,11 +1429,12 @@ void unmake_file(struct file_struct *file) static struct file_struct *send_file_name(int f, struct file_list *flist, const char *fname, STRUCT_STAT *stp, - int flags, int filter_level) + int flags, int filter_level, + unsigned int phys_depth) { struct file_struct *file; - file = make_file(fname, flist, stp, flags, filter_level); + file = make_file(fname, flist, stp, flags, filter_level, phys_depth); if (!file) return NULL; @@ -1570,7 +1588,7 @@ static void send_if_directory(int f, struct file_list *flist, return; } save_filters = push_local_filters(fbuf, len); - send_directory(f, flist, fbuf, len, flags); + send_directory(f, flist, fbuf, len, flags, F_DIR_PHYS_DEPTH(file)); pop_local_filters(save_filters); fbuf[ol] = '\0'; if (is_dot_dir) @@ -1703,7 +1721,7 @@ static void interpret_stat_error(const char *fname, int is_dir) * might call this with f set to -2, which also indicates that local filter * rules should be ignored. */ static void send_directory(int f, struct file_list *flist, char *fbuf, int len, - int flags) + int flags, unsigned int phys_depth) { struct dirent *di; unsigned remainder; @@ -1752,7 +1770,7 @@ static void send_directory(int f, struct file_list *flist, char *fbuf, int len, continue; } - send_file_name(f, flist, fbuf, NULL, flags, filter_level); + send_file_name(f, flist, fbuf, NULL, flags, filter_level, phys_depth); } fbuf[len] = '\0'; @@ -1772,6 +1790,10 @@ static void send_directory(int f, struct file_list *flist, char *fbuf, int len, } } +/* Used in a few places where we need to pass a phys_depth but can't easily + * determine what it should be. FIXME! */ +#define FIXME_path_depth path_depth + static void send_implied_dirs(int f, struct file_list *flist, char *fname, char *start, char *limit, int flags, char name_type) { @@ -1826,14 +1848,16 @@ static void send_implied_dirs(int f, struct file_list *flist, char *fname, for (slash = start; (slash = strchr(slash+1, '/')) != NULL; ) { *slash = '\0'; - file = send_file_name(f, flist, fname, NULL, flags, ALL_FILTERS); + file = send_file_name(f, flist, fname, NULL, flags, ALL_FILTERS, + FIXME_path_depth(fname)); depth++; if (!inc_recurse && file && S_ISDIR(file->mode)) change_local_filter_dir(fname, strlen(fname), depth); *slash = '/'; } - file = send_file_name(f, flist, fname, NULL, flags, ALL_FILTERS); + file = send_file_name(f, flist, fname, NULL, flags, ALL_FILTERS, + FIXME_path_depth(fname)); if (inc_recurse) { if (file && !S_ISDIR(file->mode)) file = NULL; @@ -1897,7 +1921,7 @@ static void send1extra(int f, struct file_struct *file, struct file_list *flist) } filesystem_dev = st.st_dev; } - send_directory(f, flist, fbuf, dlen, flags); + send_directory(f, flist, fbuf, dlen, flags, F_DIR_PHYS_DEPTH(file)); } if (!relative_paths) @@ -1930,9 +1954,11 @@ static void send1extra(int f, struct file_struct *file, struct file_list *flist) interpret_stat_error(fbuf, True); continue; } - send_file_name(f, flist, fbuf, &st, FLAG_TOP_DIR | flags, ALL_FILTERS); + send_file_name(f, flist, fbuf, &st, FLAG_TOP_DIR | flags, ALL_FILTERS, + FIXME_path_depth(fbuf)); } else - send_file_name(f, flist, fbuf, NULL, FLAG_TOP_DIR | flags, ALL_FILTERS); + send_file_name(f, flist, fbuf, NULL, FLAG_TOP_DIR | flags, ALL_FILTERS, + FIXME_path_depth(fbuf)); } free(relname_list); @@ -2174,7 +2200,7 @@ struct file_list *send_file_list(int f, int argc, char *argv[]) if (*fn == '.' && fn[1] == '/' && !implied_dot_dir) { send_file_name(f, flist, ".", NULL, (flags | FLAG_IMPLIED_DIR) & ~FLAG_CONTENT_DIR, - ALL_FILTERS); + ALL_FILTERS, 0); implied_dot_dir = 1; } len = clean_fname(fn, CFN_KEEP_TRAILING_SLASH @@ -2278,7 +2304,7 @@ struct file_list *send_file_list(int f, int argc, char *argv[]) struct file_struct *file; file = send_file_name(f, flist, fbuf, &st, FLAG_TOP_DIR | FLAG_CONTENT_DIR | flags, - NO_FILTERS); + NO_FILTERS, FIXME_path_depth(fbuf)); if (!file) continue; if (inc_recurse) { @@ -2287,12 +2313,13 @@ struct file_list *send_file_list(int f, int argc, char *argv[]) send_dir_depth = 0; change_local_filter_dir(fbuf, len, send_dir_depth); } - send_directory(f, flist, fbuf, len, flags); + send_directory(f, flist, fbuf, len, flags, F_DIR_PHYS_DEPTH(file)); } } else send_if_directory(f, flist, file, fbuf, len, flags); } else - send_file_name(f, flist, fbuf, &st, flags, NO_FILTERS); + send_file_name(f, flist, fbuf, &st, flags, NO_FILTERS, + FIXME_path_depth(fbuf)); } gettimeofday(&end_tv, NULL); @@ -3155,7 +3182,8 @@ struct file_list *get_dirlist(char *dirname, int dlen, int ignore_filter_rules) recurse = 0; xfer_dirs = 1; - send_directory(ignore_filter_rules ? -2 : -1, dirlist, dirname, dlen, FLAG_CONTENT_DIR); + send_directory(ignore_filter_rules ? -2 : -1, dirlist, dirname, dlen, + FLAG_CONTENT_DIR, PHYS_DEPTH_N_A); xfer_dirs = save_xfer_dirs; recurse = save_recurse; if (INFO_GTE(PROGRESS, 1)) diff --git a/generator.c b/generator.c index acf4b888..48ef16c7 100644 --- a/generator.c +++ b/generator.c @@ -1354,7 +1354,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, if (preserve_links && S_ISLNK(file->mode)) { #ifdef SUPPORT_LINKS const char *sl = F_SYMLINK(file); - if (safe_symlinks && unsafe_symlink(sl, fname)) { + if (safe_symlinks && unsafe_symlink(sl, path_depth(fname))) { if (INFO_GTE(NAME, 1)) { if (solo_file) fname = f_name(file, NULL); @@ -1644,7 +1644,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, if (inplace && make_backups > 0 && fnamecmp_type == FNAMECMP_FNAME) { if (!(backupptr = get_backup_name(fname))) goto cleanup; - if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS))) + if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS, PHYS_DEPTH_N_A))) goto pretend_missing; if (copy_file(fname, backupptr, -1, back_file->mode) < 0) { unmake_file(back_file); @@ -1682,7 +1682,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, close(fd); goto cleanup; } - if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS))) { + if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS, PHYS_DEPTH_N_A))) { close(fd); goto pretend_missing; } diff --git a/rsync.h b/rsync.h index b0ac7a9f..a68c62f4 100644 --- a/rsync.h +++ b/rsync.h @@ -726,6 +726,14 @@ extern int xattrs_ndx; #define F_SUM(f) ((char*)OPT_EXTRA(f, START_BUMP(f) + HLINK_BUMP(f) \ + SUM_EXTRA_CNT - 1)) +/* Present on all sender dirs when --copy-unsafe-links is enabled. + * See explanation at make_file. */ +#define F_DIR_PHYS_DEPTH(f) OPT_EXTRA(f, START_BUMP(f))->unum + +/* "phys_depth" arg to pass to make_file when not applicable + * (e.g., receiving side) */ +#define PHYS_DEPTH_N_A 0 + /* Some utility defines: */ #define F_IS_ACTIVE(f) (f)->basename[0] #define F_IS_HLINKED(f) ((f)->flags & FLAG_HLINKED) diff --git a/t_unsafe.c b/t_unsafe.c index 550bc4b3..12908f48 100644 --- a/t_unsafe.c +++ b/t_unsafe.c @@ -42,7 +42,7 @@ main(int argc, char **argv) } printf("%s\n", - unsafe_symlink(argv[1], argv[2]) ? "unsafe" : "safe"); + unsafe_symlink(argv[1], path_depth(argv[2])) ? "unsafe" : "safe"); return 0; } diff --git a/util.c b/util.c index 6019fccb..4e745e5a 100644 --- a/util.c +++ b/util.c @@ -1224,6 +1224,31 @@ int handle_partial_dir(const char *fname, int create) return 1; } +/* Computes the depth of the given path, excluding any basename. This + * assumes that the path does not walk through any symlinks. + * + * For use with unsafe_symlink. */ +int path_depth(const char *path) +{ + const char *name, *slash; + int depth = 0; + + for (name = path; (slash = strchr(name, '/')) != 0; name = slash+1) { + /* ".." segment starts the count over. "." segment is ignored. */ + if (strncmp(name, "./", 2) == 0) + ; + else if (strncmp(name, "../", 3) == 0) + depth = 0; + else + depth++; + while (slash[1] == '/') slash++; /* just in case path isn't clean */ + } + if (strcmp(name, "..") == 0) + depth = 0; + + return depth; +} + /* Determine if a symlink points outside the current directory tree. * This is considered "unsafe" because e.g. when mirroring somebody * else's machine it might allow them to establish a symlink to @@ -1240,47 +1265,35 @@ int handle_partial_dir(const char *fname, int create) * * "dest" is the target of the symlink in question. * - * "src" is the top source directory currently applicable at the level - * of the referenced symlink. This is usually the symlink's full path - * (including its name), as referenced from the root of the transfer. */ -int unsafe_symlink(const char *dest, const char *src) + * "depth" is the depth of the symlink inside the current directory + * tree. It can be computed with "path_depth". */ +int unsafe_symlink(const char *dest, int depth) { const char *name, *slash; - int depth = 0; /* all absolute and null symlinks are unsafe */ if (!dest || !*dest || *dest == '/') return 1; - /* find out what our safety margin is */ - for (name = src; (slash = strchr(name, '/')) != 0; name = slash+1) { - /* ".." segment starts the count over. "." segment is ignored. */ - if (*name == '.' && (name[1] == '/' || (name[1] == '.' && name[2] == '/'))) { - if (name[1] == '.') - depth = 0; - } else - depth++; - while (slash[1] == '/') slash++; /* just in case src isn't clean */ - } - if (*name == '.' && name[1] == '.' && name[2] == '\0') - depth = 0; - for (name = dest; (slash = strchr(name, '/')) != 0; name = slash+1) { - if (*name == '.' && (name[1] == '/' || (name[1] == '.' && name[2] == '/'))) { - if (name[1] == '.') { - /* if at any point we go outside the current directory - then stop - it is unsafe */ - if (--depth < 0) - return 1; - } + if (strncmp(name, "./", 2) == 0) + ; + else if (strncmp(name, "../", 3) == 0) { + /* if at any point we go outside the current directory + then stop - it is unsafe */ + if (--depth < 0) + return 1; } else + /* XXX: We could be walking through another symlink! */ depth++; while (slash[1] == '/') slash++; } - if (*name == '.' && name[1] == '.' && name[2] == '\0') - depth--; + if (strcmp(name, "..") == 0) { + if (--depth < 0) + return 1; + } - return depth < 0; + return 0; } /* Return the date and time as a string. Some callers tweak returned buf. */ -- 2.34.1