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

diff --git a/backup.c b/backup.c
index 8512e00..840bab7 100644 (file)
--- 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 a167c3b..1819e39 100644 (file)
--- 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 acf4b88..48ef16c 100644 (file)
--- 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 b0ac7a9..a68c62f 100644 (file)
--- 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 550bc4b..12908f4 100644 (file)
--- 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 6019fcc..4e745e5 100644 (file)
--- 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. */