Add a new daemon security option: "munge symlinks".

clientserver.c

@@ -58,6 +58,7 @@ extern char curr_dir[];
 char *auth_user;
 int read_only = 0;
 int module_id = -1;
+int munge_symlinks = 0;
 struct chmod_mode_struct *daemon_chmod_modes;
 
 /* module_dirlen is the length of the module_dir string when in daemon
@@ -624,6 +625,18 @@ static int rsync_module(int f_in, int f_out, int i, char *addr, char *host)
 		sanitize_paths = 1;
 	}
 
+	if ((munge_symlinks = lp_munge_symlinks(i)) < 0)
+		munge_symlinks = !use_chroot;
+	if (munge_symlinks) {
+		STRUCT_STAT st;
+		if (stat(SYMLINK_PREFIX, &st) == 0 && S_ISDIR(st.st_mode)) {
+			rprintf(FLOG, "Symlink munging is unsupported when a %s directory exists.\n",
+				SYMLINK_PREFIX);
+			io_printf(f_out, "@ERROR: daemon security issue -- contact admin\n", name);
+			exit_cleanup(RERR_UNSUPPORTED);
+		}
+	}
+
 	if (am_root) {
 		/* XXXX: You could argue that if the daemon is started
 		 * by a non-root user and they explicitly specify a

flist.c

@@ -63,6 +63,7 @@ extern int copy_links;
 extern int copy_unsafe_links;
 extern int protocol_version;
 extern int sanitize_paths;
+extern int munge_symlinks;
 extern int need_unsorted_flist;
 extern int unsort_ndx;
 extern struct stats stats;
@@ -200,6 +201,11 @@ static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf)
 			}
 			return x_stat(path, stp, NULL);
 		}
+		if (munge_symlinks && am_sender && llen > SYMLINK_PREFIX_LEN
+		 && strncmp(linkbuf, SYMLINK_PREFIX, SYMLINK_PREFIX_LEN) == 0) {
+			memmove(linkbuf, linkbuf + SYMLINK_PREFIX_LEN,
+				llen - SYMLINK_PREFIX_LEN + 1);
+		}
 	}
 	return 0;
 #else
@@ -794,6 +800,8 @@ static struct file_struct *recv_file_entry(struct file_list *flist,
 				linkname_len - 1);
 			overflow_exit("recv_file_entry");
 		}
+		if (munge_symlinks)
+			linkname_len += SYMLINK_PREFIX_LEN;
 	}
 	else
 #endif
@@ -914,10 +922,13 @@ static struct file_struct *recv_file_entry(struct file_list *flist,
 		if (first_hlink_ndx >= flist->ndx_start) {
 			struct file_struct *first = flist->files[first_hlink_ndx - flist->ndx_start];
 			memcpy(bp, F_SYMLINK(first), linkname_len);
+		} else if (munge_symlinks) {
+			strlcpy(bp, SYMLINK_PREFIX, linkname_len);
+			bp += SYMLINK_PREFIX_LEN;
+			linkname_len -= SYMLINK_PREFIX_LEN;
+			read_sbuf(f, bp, linkname_len - 1);
 		} else
 			read_sbuf(f, bp, linkname_len - 1);
-		if (sanitize_paths)
-			sanitize_path(bp, bp, "", lastdir_depth);
 	}
 #endif
 

loadparm.c

@@ -157,6 +157,7 @@ typedef struct
 	BOOL ignore_errors;
 	BOOL ignore_nonreadable;
 	BOOL list;
+	BOOL munge_symlinks;
 	BOOL read_only;
 	BOOL strict_modes;
 	BOOL transfer_logging;
@@ -205,6 +206,7 @@ static service sDefault =
  /* ignore_errors; */		False,
  /* ignore_nonreadable; */	False,
  /* list; */			True,
+ /* munge_symlinks; */		(BOOL)-1,
  /* read_only; */		True,
  /* strict_modes; */		True,
  /* transfer_logging; */	False,
@@ -319,6 +321,7 @@ static struct parm_struct parm_table[] =
  {"log format",        P_STRING, P_LOCAL, &sDefault.log_format,        NULL,0},
  {"max connections",   P_INTEGER,P_LOCAL, &sDefault.max_connections,   NULL,0},
  {"max verbosity",     P_INTEGER,P_LOCAL, &sDefault.max_verbosity,     NULL,0},
+ {"munge symlinks",    P_BOOL,   P_LOCAL, &sDefault.munge_symlinks,    NULL,0},
  {"name",              P_STRING, P_LOCAL, &sDefault.name,              NULL,0},
  {"outgoing chmod",    P_STRING, P_LOCAL, &sDefault.outgoing_chmod,    NULL,0},
  {"path",              P_PATH,   P_LOCAL, &sDefault.path,              NULL,0},
@@ -422,6 +425,7 @@ FN_LOCAL_BOOL(lp_fake_super, fake_super)
 FN_LOCAL_BOOL(lp_ignore_errors, ignore_errors)
 FN_LOCAL_BOOL(lp_ignore_nonreadable, ignore_nonreadable)
 FN_LOCAL_BOOL(lp_list, list)
+FN_LOCAL_BOOL(lp_munge_symlinks, munge_symlinks)
 FN_LOCAL_BOOL(lp_read_only, read_only)
 FN_LOCAL_BOOL(lp_strict_modes, strict_modes)
 FN_LOCAL_BOOL(lp_transfer_logging, transfer_logging)

rsync.h

@@ -32,6 +32,9 @@
 #define DEFAULT_LOCK_FILE "/var/run/rsyncd.lock"
 #define URL_PREFIX "rsync://"
 
+#define SYMLINK_PREFIX "/rsyncd-munged/"
+#define SYMLINK_PREFIX_LEN ((int)sizeof SYMLINK_PREFIX - 1)
+
 #define BACKUP_SUFFIX "~"
 
 /* a non-zero CHAR_OFFSET makes the rolling sum stronger, but is

rsyncd.conf.yo

@@ -130,12 +130,15 @@ the advantage of extra protection against possible implementation security
 holes, but it has the disadvantages of requiring super-user privileges,
 of not being able to follow symbolic links that are either absolute or outside
 of the new root path, and of complicating the preservation of usernames and groups
-(see below).  When "use chroot" is false, for security reasons,
-symlinks may only be relative paths pointing to other files within the root
-path, and leading slashes are removed from most absolute paths (options
-such as bf(--backup-dir), bf(--compare-dest), etc. interpret an absolute path as
-rooted in the module's "path" dir, just as if chroot was specified).
-The default for "use chroot" is true.
+(see below).  When "use chroot" is false, rsync will: (1) munge symlinks by
+default for security reasons (see "munge symlinks" for a way to turn this
+off, but only if you trust your users), (2) substitute leading slashes in
+absolute paths with the module's path (so that options such as
+bf(--backup-dir), bf(--compare-dest), etc. interpret an absolute path as
+rooted in the module's "path" dir), and (3) trim ".." path elements from
+args if rsync believes they would escape the chroot.
+The default for "use chroot" is true, and is the safer choice (especially
+if the module is not read-only).
 
 In order to preserve usernames and groupnames, rsync needs to be able to
 use the standard library functions for looking up names and IDs (i.e.
@@ -159,6 +162,34 @@ access to some of the excluded files inside the directory (rsync tries to
 do this automatically, but you might as well specify both to be extra
 sure).
 
+dit(bf(munge symlinks))  The "munge symlinks" option tells rsync to modify
+all incoming symlinks in a way that makes them unusable but recoverable
+(see below).  This should help protect your files from user trickery when
+your daemon module is writable.  The default is disabled when "use chroot"
+is on and enabled when "use chroot" is off.
+
+If you disable this option on a daemon that is not read-only, there
+are tricks that a user can play with uploaded symlinks to access
+daemon-excluded items (if your module has any), and, if "use chroot"
+is off, rsync can even be tricked into showing or changing data that
+is outside the module's path (as access-permissions allow).
+
+The way rsync disables the use of symlinks is to prefix each one with
+the string "/rsyncd-munged/".  This prevents the links from being used
+as long as that directory does not exist.  When this option is enabled,
+rsync will refuse to run if that path is a directory or a symlink to
+a directory.  When using the "munge symlinks" option in a chroot area,
+you should add this path to the exclude setting for the module so that
+the user can't try to create it.
+
+Note:  rsync makes no attempt to verify that any pre-existing symlinks in
+the hierarchy are as safe as you want them to be.  If you setup an rsync
+daemon on a new area or locally add symlinks, you can manually protect your
+symlinks from being abused by prefixing "/rsyncd-munged/" to the start of
+every symlink's value.  There is a perl script in the support directory
+of the source code named "munge-symlinks" that can be used to add or remove
+this prefix from your symlinks.
+
 dit(bf(max connections)) The "max connections" option allows you to
 specify the maximum number of simultaneous connections you will allow.
 Any clients connecting when the maximum has been reached will receive a

support/munge-symlinks

@@ -0,0 +1,60 @@
+#!/usr/bin/perl
+# This script will either prefix all symlink values with the string
+# "/rsyncd-munged/" or remove that prefix.
+
+use strict;
+use Getopt::Long;
+
+my $SYMLINK_PREFIX = '/rsyncd-munged/';
+
+my $munge_opt;
+
+&GetOptions(
+    'munge' => sub { $munge_opt = 1 },
+    'unmunge' => sub { $munge_opt = 0 },
+    'all' => \( my $all_opt ),
+    'help|h' => \( my $help_opt ),
+) or &usage;
+
+&usage if $help_opt || !defined $munge_opt;
+
+my $munged_re = $all_opt ? qr/^($SYMLINK_PREFIX)+(?=.)/ : qr/^$SYMLINK_PREFIX(?=.)/;
+
+push(@ARGV, '.') unless @ARGV;
+
+open(PIPE, '-|', 'find', @ARGV, '-type', 'l') or die $!;
+
+while (<PIPE>) {
+    chomp;
+    my $lnk = readlink($_) or next;
+    if ($munge_opt) {
+	next if !$all_opt && $lnk =~ /$munged_re/;
+	$lnk =~ s/^/$SYMLINK_PREFIX/;
+    } else {
+	next unless $lnk =~ s/$munged_re//;
+    }
+    if (!unlink($_)) {
+	warn "Unable to unlink symlink: $_ ($!)\n";
+    } elsif (!symlink($lnk, $_)) {
+	warn "Unable to recreate symlink: $_ -> $lnk ($!)\n";
+    } else {
+	print "$_ -> $lnk\n";
+    }
+}
+
+close PIPE;
+exit;
+
+sub usage
+{
+    die <<EOT;
+Usage: munge-symlinks --munge|--unmunge [--all] [DIR|SYMLINK...]
+
+--munge      Add the $SYMLINK_PREFIX prefix to symlinks if not already
+             present, or always when combined with --all.
+--unmunge    Remove one $SYMLINK_PREFIX prefix from symlinks or all
+             such prefixes with --all.
+
+See the "munge symlinks" option in the rsyncd.conf manpage for more details.
+EOT
+}

testsuite/rsync.fns

@@ -234,6 +234,7 @@ build_rsyncd_conf() {
 
 pid file = $pidfile
 use chroot = no
+munge symlinks = no
 hosts allow = localhost 127.0.0.0/24 192.168.0.0/16 10.0.0.0/8 $hostname
 log file = $logfile
 log format = %i %h [%a] %m (%u) %l %f%L