daemon: robustness against malicious-peer protocol input

Refuse malformed/hostile wire input that could crash or corrupt the receiver:
io.c (out-of-range file index, count*blength OFF_T overflow, read_args NUL room,
deferred in_multiplexed), flist.c (sub-flist after the final flist is freed,
FLAG_HLINKED on dirs / gated on preserve_hard_links, parent_ndx bound, cleared-
slot ndx in the transfer phase), hlink.c (undeclared cross-flist gnum -> error not
assert), match.c (clamp peer flength, re-check len before want_i), log.c (drop
peer-reachable asserts and F_SUM deref), exclude.c (merge-file recursion cap,
trailing-backslash heap fix), lib/pool_alloc.c (ASan redzone for pool underflow).

exclude.c

@@ -71,6 +71,9 @@ static int dirbuf_depth;
 /* This is True when we're scanning parent dirs for per-dir merge-files. */
 static BOOL parent_dirscan = False;
 
+#define MAX_MERGE_DEPTH 32
+static int merge_depth = 0;
+
 /* This array contains a list of all the currently active per-dir merge
  * files.  This makes it easier to save the appropriate values when we
  * "push" down into each subdirectory. */
@@ -427,7 +430,7 @@ void add_implied_include(const char *arg, int skip_daemon_module)
 				if (cp[1] == ']') {
 					if (!saw_wild)
 						cp++; /* A \] in a non-wild filter causes a problem, so drop the \ . */
-				} else if (!strchr("*[?", cp[1])) {
+				} else if (!cp[1] || !strchr("*[?", cp[1])) {
 					backslash_cnt++;
 					if (saw_wild)
 						*p++ = '\\';
@@ -1033,6 +1036,41 @@ int check_server_filter(filter_rule_list *listp, enum logcode code, const char *
 	return ret;
 }
 
+/* Returns 1 if `name` matches an implied-parent rule (a directory component
+ * seeded by add_implied_include() with FILTRULE_DIRECTORY) but not a leaf
+ * rule -- i.e. the client asked for something under the dir, never the dir
+ * itself as content.
+ *
+ * The receiver uses this to refuse a malicious sender that sets XMIT_TOP_DIR
+ * without XMIT_NO_CONTENT_DIR on such a dir: the honest encoding is both flags
+ * (flist.c send path), so otherwise the receiver would set FLAG_CONTENT_DIR
+ * and delete_in_dir() could sweep pre-existing siblings under --delete. */
+int is_implied_parent_dir(const char *name)
+{
+	filter_rule *ent;
+	int parent_match = 0;
+
+	if (!implied_filter_list.head)
+		return 0;
+
+	for (ent = implied_filter_list.head; ent; ent = ent->next) {
+		if (ent->rflags & (FILTRULE_PERDIR_MERGE | FILTRULE_CVS_IGNORE))
+			continue;
+		if (!rule_matches(name, ent, NAME_IS_DIR))
+			continue;
+		if (!(ent->rflags & FILTRULE_INCLUDE))
+			continue;
+		if (ent->rflags & FILTRULE_DIRECTORY) {
+			parent_match = 1;
+			continue;
+		}
+		/* A non-DIRECTORY include rule = a leaf the client asked for, so
+		 * the dir is legitimately in the list, not parent-only. */
+		return 0;
+	}
+	return parent_match;
+}
+
 /* Return -1 if file "name" is defined to be excluded by the specified
  * exclude list, 1 if it is included, and 0 if it was not matched. */
 int check_filter(filter_rule_list *listp, enum logcode code,
@@ -1454,16 +1492,57 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_
 	if (!fname || !*fname)
 		return;
 
+	if (merge_depth >= MAX_MERGE_DEPTH) {
+		rprintf(FERROR,
+			"[%s] merge-file include depth limit (%d) exceeded at %s\n",
+			who_am_i(), MAX_MERGE_DEPTH, fname);
+		/* Match the failed-open path below: abort under a fatal
+		 * (operator-supplied) merge, otherwise drop the rule. */
+		if (xflags & XFLG_FATAL_ERRORS)
+			exit_cleanup(RERR_FILEIO);
+		return;
+	}
+	merge_depth++;
+
 	if (*fname != '-' || fname[1] || am_server) {
+		/* This path is operator- and (via per-directory merge files like
+		 * .cvsignore) sender-controlled: a planted symlink could leak a
+		 * root-readable file through the filter parser, or redirect an
+		 * --exclude-from open via a planted parent.  Refuse symlinks not
+		 * owned by uid 0 or our euid. */
+		const char *open_path;
+		int fd;
 		if (daemon_filter_list.head) {
+			char *dir;
 			strlcpy(line, fname, sizeof line);
-			clean_fname(line, CFN_COLLAPSE_DOT_DOT_DIRS);
-			if (check_filter(&daemon_filter_list, FLOG, line, 0) < 0)
-				fp = NULL;
-			else
-				fp = fopen(line, "rb");
+			/* parse_merge_name() prepends module_dir for absolute paths,
+			 * so strip module_dirlen back off before the check or the
+			 * anchored module-relative daemon rule won't match (as
+			 * options.c does for --exclude-from/--include-from).  The
+			 * original absolute path is still used for the open below. */
+			dir = line + (*line == '/' ? module_dirlen : 0);
+			clean_fname(dir, CFN_COLLAPSE_DOT_DOT_DIRS);
+			if (check_filter(&daemon_filter_list, FLOG, dir, 0) < 0) {
+				/* Hidden by the daemon filter: treat the merge file as
+				 * non-existent rather than tripping XFLG_FATAL_ERRORS
+				 * below, so it neither errors out nor leaks a
+				 * fatal-vs-silent oracle. */
+				if (DEBUG_GTE(FILTER, 2)) {
+					rprintf(FINFO,
+						"[%s] parse_filter_file(%s) hidden by daemon filter\n",
+						who_am_i(), fname);
+				}
+				merge_depth--;
+				return;
+			}
+			open_path = line;
 		} else
-			fp = fopen(fname, "rb");
+			open_path = fname;
+		fd = open_no_attacker_symlinks(open_path, O_RDONLY, 0);
+		if (fd < 0)
+			fp = NULL;
+		else if (!(fp = fdopen(fd, "rb")))
+			close(fd);
 	} else
 		fp = stdin;
 
@@ -1481,6 +1560,7 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_
 				fname);
 			exit_cleanup(RERR_FILEIO);
 		}
+		merge_depth--;
 		return;
 	}
 	dirbuf[dirbuf_len] = '\0';
@@ -1517,6 +1597,7 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_
 			break;
 	}
 	fclose(fp);
+	merge_depth--;
 }
 
 /* If the "for_xfer" flag is set, the prefix is made compatible with the

flist.c

@@ -29,6 +29,11 @@
 extern int am_root;
 extern int am_server;
 extern int am_daemon;
+extern int am_chrooted;
+extern char *module_dir;
+extern unsigned int module_dirlen;
+extern int module_dirfd;
+extern unsigned int curr_dir_len;
 extern int am_sender;
 extern int am_generator;
 extern int inc_recurse;
@@ -64,6 +69,7 @@ extern int non_perishable_cnt;
 extern int prune_empty_dirs;
 extern int copy_links;
 extern int copy_unsafe_links;
+extern int insecure_links;
 extern int protocol_version;
 extern int sanitize_paths;
 extern int munge_symlinks;
@@ -214,13 +220,47 @@ 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. */
+/* Set by send_directory() to the fd of the directory it is currently scanning
+ * (and that dir's path prefix), so the per-entry stat can go through the
+ * already-open dir fd instead of re-resolving the full path for every entry.
+ * Pure performance and sender-side only -- the scanned dir is already open, so
+ * fstatat(scan_dirfd, basename) is identical to lstat(scandir/basename); no
+ * confinement is implied or needed. */
+static int scan_dirfd = -1;
+static const char *scan_dir_prefix;
+static int scan_dir_prefix_len;
+
+static int scan_link_stat(const char *path, STRUCT_STAT *stp, int follow_dirlinks)
+{
+	/* Use the held scan fd only for a single component directly inside the
+	 * scanned dir, and only when am_root >= 0 (link_stat_at folds in no
+	 * fake-super %stat xattr; link_stat does so via get_stat_xattr, a no-op
+	 * once am_root >= 0). */
+	if (scan_dirfd >= 0 && am_root >= 0
+	 && strncmp(path, scan_dir_prefix, scan_dir_prefix_len) == 0
+	 && path[scan_dir_prefix_len] == '/'
+	 && strchr(path + scan_dir_prefix_len + 1, '/') == NULL)
+		return link_stat_at(scan_dirfd, path + scan_dir_prefix_len + 1, stp, follow_dirlinks);
+	return link_stat(path, stp, follow_dirlinks);
+}
+
+static int scan_readlink(const char *path, char *linkbuf, size_t bufsiz)
+{
+	if (scan_dirfd >= 0 && am_root >= 0
+	 && strncmp(path, scan_dir_prefix, scan_dir_prefix_len) == 0
+	 && path[scan_dir_prefix_len] == '/'
+	 && strchr(path + scan_dir_prefix_len + 1, '/') == NULL)
+		return do_readlink_atfd(scan_dirfd, path + scan_dir_prefix_len + 1, linkbuf, bufsiz);
+	return do_readlink(path, linkbuf, bufsiz);
+}
+
 static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf)
 {
 #ifdef SUPPORT_LINKS
-	if (link_stat(path, stp, copy_dirlinks) < 0)
+	if (scan_link_stat(path, stp, copy_dirlinks) < 0)
 		return -1;
 	if (S_ISLNK(stp->st_mode)) {
-		int llen = do_readlink(path, linkbuf, MAXPATHLEN - 1);
+		int llen = scan_readlink(path, linkbuf, MAXPATHLEN - 1);
 		if (llen < 0)
 			return -1;
 		linkbuf[llen] = '\0';
@@ -261,6 +301,30 @@ int link_stat(const char *path, STRUCT_STAT *stp, int follow_dirlinks)
 #endif
 }
 
+/* Held-dirfd variant of link_stat(): stat single-component `name` relative to
+ * directory fd `dfd`, instead of re-resolving a full path.  Equivalent to
+ * link_stat() only when NOT in --fake-super mode -- x_stat/x_lstat fold the
+ * fake-super %stat xattr into the result via get_stat_xattr(), which is a
+ * path-based no-op once am_root >= 0.  Callers therefore use this only when
+ * am_root >= 0 (and a valid dfd), falling back to link_stat() otherwise. */
+int link_stat_at(int dfd, const char *name, STRUCT_STAT *stp, int follow_dirlinks)
+{
+#ifdef SUPPORT_LINKS
+	if (copy_links)
+		return do_stat_atfd(dfd, name, stp);
+	if (do_lstat_atfd(dfd, name, stp) < 0)
+		return -1;
+	if (follow_dirlinks && S_ISLNK(stp->st_mode)) {
+		STRUCT_STAT st;
+		if (do_stat_atfd(dfd, name, &st) == 0 && S_ISDIR(st.st_mode))
+			*stp = st;
+	}
+	return 0;
+#else
+	return do_stat_atfd(dfd, name, stp);
+#endif
+}
+
 static inline int path_is_daemon_excluded(char *path, int ignore_filename)
 {
 	if (daemon_filter_list.head) {
@@ -776,7 +840,7 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
 
 	if ((basename = strrchr(thisname, '/')) != NULL) {
 		int len = basename++ - thisname;
-		if (len != lastdir_len || memcmp(thisname, lastdir, len) != 0) {
+		if (len != lastdir_len || !lastdir || memcmp(thisname, lastdir, len) != 0) {
 			lastdir = new_array(char, len + 1);
 			memcpy(lastdir, thisname, len);
 			lastdir[len] = '\0';
@@ -1055,7 +1119,8 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
 	memcpy(bp, basename, basename_len);
 
 #ifdef SUPPORT_HARD_LINKS
-	if (xflags & XMIT_HLINKED
+	if (preserve_hard_links && xflags & XMIT_HLINKED
+	 && !S_ISDIR(mode)
 #ifndef CAN_HARDLINK_SYMLINK
 	 && !S_ISLNK(mode)
 #endif
@@ -1110,6 +1175,26 @@ static struct file_struct *recv_file_entry(int f, struct file_list *flist, int x
 		if (basename_len == 1+1 && *basename == '.') /* +1 for '\0' */
 			F_DEPTH(file)--;
 		if (protocol_version >= 30) {
+			/* Stop a malicious sender expanding --delete scope by flagging
+			 * an implied parent as a content dir: if we only allowed this
+			 * entry as a parent of the requested leaf, force the flags back
+			 * to the honest implied-parent encoding (XMIT_TOP_DIR |
+			 * XMIT_NO_CONTENT_DIR) so it lands in FLAG_IMPLIED_DIR, not
+			 * FLAG_CONTENT_DIR, and delete_in_dir() can't sweep siblings.
+			 * Not gated on trust_sender_filter: implied_filter_list is
+			 * receiver-owned state, so a per-dir filter must not be able to
+			 * downgrade this defense. */
+			if (implied_filter_list.head
+			 && is_implied_parent_dir(thisname)
+			 && (!(xflags & XMIT_NO_CONTENT_DIR) || !(xflags & XMIT_TOP_DIR))) {
+				if (DEBUG_GTE(FILTER, 1)) {
+					rprintf(FINFO,
+						"[%s] receiver downgraded implied-parent dir %s "
+						"to non-content (sender xflags=0x%x)\n",
+						who_am_i(), thisname, xflags);
+				}
+				xflags |= XMIT_NO_CONTENT_DIR | XMIT_TOP_DIR;
+			}
 			if (!(xflags & XMIT_NO_CONTENT_DIR)) {
 				if (xflags & XMIT_TOP_DIR)
 					file->flags |= FLAG_TOP_DIR;
@@ -1400,7 +1485,7 @@ struct file_struct *make_file(const char *fname, struct file_list *flist,
 
 	if ((basename = strrchr(thisname, '/')) != NULL) {
 		int len = basename++ - thisname;
-		if (len != lastdir_len || memcmp(thisname, lastdir, len) != 0) {
+		if (len != lastdir_len || !lastdir || memcmp(thisname, lastdir, len) != 0) {
 			lastdir = new_array(char, len + 1);
 			memcpy(lastdir, thisname, len);
 			lastdir[len] = '\0';
@@ -1848,6 +1933,77 @@ static void interpret_stat_error(const char *fname, int is_dir)
 	}
 }
 
+#if defined HAVE_FDOPENDIR && defined HAVE_DIRFD
+/* Open a source directory for scanning confined beneath the transfer root.
+ * secure_relative_open() does a per-component O_NOFOLLOW walk that refuses a
+ * parent component raced into a symlink pointing out of the tree; fdopendir()
+ * then turns the held fd into the DIR* the scan reads.  This mirrors the
+ * sender's confined content open (sender.c): the directory enumeration must be
+ * confined the same way, or a parent-symlink race (or, for a daemon following
+ * mode, an in-module symlink to outside) lets the scan enumerate an out-of-tree
+ * directory and leak its names/metadata/symlink targets.  O_DIRECTORY without
+ * O_NOFOLLOW makes secure_relative_open() follow in-tree directory symlinks
+ * beneath the anchor and refuse escapes, so this serves both the default
+ * no-follow scan and a daemon's symlink-following scan (see the caller).
+ * Returns NULL with errno set on failure, like opendir(). */
+static DIR *secure_opendir(const char *fbuf)
+{
+	int dfd, fl;
+	DIR *d;
+
+	if (am_daemon && (!am_chrooted || module_dirlen)
+	 && module_dir && module_dir[0] == '/' && *fbuf != '/' && module_dirfd >= 0
+	 && curr_dir_len >= module_dirlen
+	 && strncmp(curr_dir, module_dir, module_dirlen) == 0
+	 && (curr_dir[module_dirlen] == '\0' || curr_dir[module_dirlen] == '/')) {
+		/* Daemon: anchor the confined scan at the module root pinned by identity
+		 * at module setup (module_dirfd, opened while the daemon was positioned
+		 * there and still privileged), and walk the module-relative path of the
+		 * scan target beneath it.  This re-follows the same in-module path -- so a
+		 * legitimate in-module ".." climb (sub/climb -> ../sibling) or an in-module
+		 * directory symlink is followed, and an escape refused -- without
+		 * re-walking the absolute module path as the dropped uid (the privilege-
+		 * drop EACCES), and without assuming the lexical curr_dir depth matches the
+		 * real cwd (a followed in-module symlink can desync them; anchoring at the
+		 * pinned module root and walking down the logical path is correct either
+		 * way). */
+		const char *p = curr_dir + module_dirlen;
+		char modrel[MAXPATHLEN];
+		while (*p == '/')
+			p++;
+		if ((size_t)snprintf(modrel, sizeof modrel, "%s%s%s",
+				     p, *p ? "/" : "", fbuf) >= sizeof modrel) {
+			errno = ENAMETOOLONG;
+			return NULL;
+		}
+		dfd = secure_relative_open_at(module_dirfd, *modrel ? modrel : ".",
+					      O_RDONLY | O_DIRECTORY, 0);
+	} else if (*fbuf == '/') {
+		/* An absolute scan path (an absolute --relative / --files-from name, or a
+		 * "/" transfer root): anchor at "/" -- operator-named, trusted. */
+		const char *relp = fbuf;
+		while (*relp == '/')
+			relp++;
+		dfd = secure_relative_open("/", relp, O_RDONLY | O_DIRECTORY, 0);
+	} else {
+		/* Non-daemon (or chrooted) sender: confine beneath the cwd the sender
+		 * chdir'd into (the transfer root). */
+		dfd = secure_relative_open(NULL, fbuf, O_RDONLY | O_DIRECTORY, 0);
+	}
+
+	if (dfd < 0)
+		return NULL;
+	if ((fl = fcntl(dfd, F_GETFD)) >= 0)
+		fcntl(dfd, F_SETFD, fl | FD_CLOEXEC);
+	if (!(d = fdopendir(dfd))) {
+		int save = errno;
+		close(dfd);
+		errno = save;
+	}
+	return d;
+}
+#endif
+
 /* This function is normally called by the sender, but the receiving side also
  * calls it from get_dirlist() with f set to -1 so that we just construct the
  * file list in memory without sending it over the wire.  Also, get_dirlist()
@@ -1866,7 +2022,31 @@ static void send_directory(int f, struct file_list *flist, char *fbuf, int len,
 
 	assert(flist != NULL);
 
-	if (!(d = opendir(fbuf))) {
+#if defined HAVE_FDOPENDIR && defined HAVE_DIRFD
+	/* Confine the enumeration beneath the transfer root.  secure_opendir()
+	 * follows in-tree directory symlinks (RESOLVE_BENEATH) and refuses one that
+	 * escapes, so it serves both modes:
+	 *   - a daemon/hardened sender (secure_relpath_active()) is confined to the
+	 *     module in EVERY mode -- including -L/--copy-dirlinks/--copy-unsafe-
+	 *     links, matching the content open (sender_open_copylinks_confined) --
+	 *     so a following mode cannot be lured to enumerate outside the module;
+	 *   - a non-daemon sender is confined in the default no-follow mode; its
+	 *     symlink-following modes intentionally dereference out of the
+	 *     operator's own tree, so they keep the legacy opendir().
+	 * f >= 0 is the sender's outgoing scan; get_dirlist() passes f < 0 and keeps
+	 * the legacy opendir().  A module opted out of confinement ("insecure links =
+	 * yes", admin-only) -- or a non-daemon --insecure-links -- uses the legacy
+	 * opendir() too, restoring the pre-hardening enumeration (re-opening the
+	 * escape; documented). */
+	if (f >= 0 && !symlink_optout_allowed() && (secure_relpath_active()
+		    || !(copy_links || copy_unsafe_links || copy_dirlinks || insecure_links)))
+		d = secure_opendir(fbuf);
+	else
+		d = opendir(fbuf);
+#else
+	d = opendir(fbuf);
+#endif
+	if (!d) {
 		if (errno == ENOENT) {
 			if (am_sender) /* Can abuse this for vanished error w/ENOENT: */
 				interpret_stat_error(fbuf, True);
@@ -1889,6 +2069,14 @@ static void send_directory(int f, struct file_list *flist, char *fbuf, int len,
 	} else
 		remainder = 0;
 
+#ifdef HAVE_DIRFD
+	/* Let the per-entry stat (readlink_stat -> scan_link_stat) go through the
+	 * already-open directory fd instead of re-resolving fbuf for each name. */
+	scan_dirfd = dirfd(d);
+	scan_dir_prefix = fbuf;
+	scan_dir_prefix_len = len;
+#endif
+
 	for (errno = 0, di = readdir(d); di; errno = 0, di = readdir(d)) {
 		unsigned name_len;
 		char *dname = d_name(di);
@@ -1917,6 +2105,9 @@ static void send_directory(int f, struct file_list *flist, char *fbuf, int len,
 		send_file_name(f, flist, fbuf, NULL, flags, filter_level);
 	}
 
+	scan_dirfd = -1;	/* fbuf is about to be reused / d closed */
+	scan_dir_prefix = NULL;	/* and don't leave the global pointing into fbuf */
+	scan_dir_prefix_len = 0;
 	fbuf[len] = '\0';
 
 	if (errno) {
@@ -2050,7 +2241,8 @@ static void send1extra(int f, struct file_struct *file, struct file_list *flist)
 	int len, dlen, flags = FLAG_DIVERT_DIRS | FLAG_CONTENT_DIR;
 	size_t j;
 
-	f_name(file, fbuf);
+	if (!f_name(file, fbuf))
+		return;
 	dlen = strlen(fbuf);
 
 	if (!change_pathname(file, NULL, 0))
@@ -2620,11 +2812,33 @@ struct file_list *recv_file_list(int f, int dir_ndx)
 #endif
 
 	if (inc_recurse && dir_ndx >= 0) {
+		if (!first_flist) {
+			/* All flists have already been freed via the NDX_DONE
+			 * chain, so dir_flist is stale: its files[] entries
+			 * point into a destroyed pool.  A sub-flist marker now
+			 * is a protocol violation (and would otherwise UAF the
+			 * stale dir entry below, then deref an uninitialised
+			 * slot in the freshly reset dir_flist further down). */
+			rprintf(FERROR_XFER,
+				"rsync: refusing sub-flist after final flist was freed\n");
+			exit_cleanup(RERR_PROTOCOL);
+		}
 		if (dir_ndx >= dir_flist->used) {
 			rprintf(FERROR_XFER, "rsync: refusing invalid dir_ndx %u >= %u\n", dir_ndx, dir_flist->used);
 			exit_cleanup(RERR_PROTOCOL);
 		}
 		struct file_struct *file = dir_flist->files[dir_ndx];
+		if (!F_IS_ACTIVE(file)) {
+			/* flist_sort_and_clean() can clear_file() a directory
+			 * entry that was a duplicate or otherwise pruned, but
+			 * the cleared file_struct stays in dir_flist.  A peer
+			 * that then sends a sub-flist for that slot would make
+			 * f_name() return NULL into the dirname strcmp() below. */
+			rprintf(FERROR_XFER,
+				"rsync: refusing flist for cleared dir_ndx %d\n",
+				dir_ndx);
+			exit_cleanup(RERR_PROTOCOL);
+		}
 		if (file->flags & FLAG_GOT_DIR_FLIST) {
 			rprintf(FERROR_XFER, "rsync: refusing malicious duplicate flist for dir %d\n", dir_ndx);
 			exit_cleanup(RERR_PROTOCOL);
@@ -2686,7 +2900,7 @@ struct file_list *recv_file_list(int f, int dir_ndx)
 				cur_dir++;
 			if (cur_dir != good_dirname) {
 				const char *d = dir_ndx >= 0 ? f_name(dir_flist->files[dir_ndx], NULL) : empty_dir;
-				if (strcmp(cur_dir, d) != 0) {
+				if (!d || strcmp(cur_dir, d) != 0) {
 					rprintf(FERROR,
 						"ABORTING due to invalid path from sender: %s/%s\n",
 						cur_dir, file->basename);
@@ -2776,7 +2990,15 @@ struct file_list *recv_file_list(int f, int dir_ndx)
 		if (!ignore_errors)
 			io_error |= err;
 	} else if (inc_recurse && flist->ndx_start == 1) {
-		if (!file_total || strcmp(flist->sorted[flist->low]->basename, ".") != 0)
+		/* The first inc_recurse flist has no parent in dir_flist; a
+		 * malicious peer can send a "." entry whose mode is not a
+		 * directory, so it never lands in dir_flist (used stays 0) yet
+		 * the basename test below still passes.  That left parent_ndx at
+		 * its default 0 and the consumers dereferenced dir_flist->files[0]
+		 * = uninitialised heap.  Require dir_flist to actually hold an
+		 * entry before trusting index 0. */
+		if (!file_total || !dir_flist->used
+		 || strcmp(flist->sorted[flist->low]->basename, ".") != 0)
 			flist->parent_ndx = -1;
 	}
 

hlink.c

@@ -125,8 +125,22 @@ static void match_gnums(int32 *ndx_list, int ndx_count)
 		if (inc_recurse) {
 			node = hashtable_find(prior_hlinks, gnum, data_when_new);
 			if (node->data == data_when_new) {
+				if (gnum < hlink_flist->ndx_start) {
+					/* A non-first hard-link entry whose
+					 * gnum points before this flist's
+					 * ndx_start should already have been
+					 * recorded in prior_hlinks by an
+					 * earlier flist.  A peer that sends
+					 * such a back-reference on the first
+					 * flist (or to a gnum that was never
+					 * declared XMIT_HLINK_FIRST) is
+					 * misbehaving. */
+					rprintf(FERROR,
+					    "hard-link gnum %d precedes flist start %d\n",
+					    (int)gnum, (int)hlink_flist->ndx_start);
+					exit_cleanup(RERR_PROTOCOL);
+				}
 				node->data = new_array0(char, 5);
-				assert(gnum >= hlink_flist->ndx_start);
 				file->flags |= FLAG_HLINK_FIRST;
 				prev = -1;
 			} else if (CVAL(node->data, 0) == 0) {

io.c

@@ -1076,11 +1076,17 @@ void send_msg_success(const char *fname, int num)
 		if (DEBUG_GTE(IO, 1))
 			rprintf(FINFO, "[%s] send_msg_success(%d)\n", who_am_i(), num);
 
-		if (stat(fname, &st) < 0)
-			memset(&st, 0, sizeof (STRUCT_STAT));
+		/* The dev/ino is consumed only by the sender's --remove-source-files
+		 * same-file safety check (successful_send), so skip the per-file
+		 * stat entirely otherwise -- it's sent but never read. */
+		if (remove_source_files && stat(fname, &st) == 0) {
+			SIVAL64(num_dev_ino_buf, 4, st.st_dev);
+			SIVAL64(num_dev_ino_buf, 4+8, st.st_ino);
+		} else {
+			SIVAL64(num_dev_ino_buf, 4, 0);
+			SIVAL64(num_dev_ino_buf, 4+8, 0);
+		}
 		SIVAL(num_dev_ino_buf, 0, num);
-		SIVAL64(num_dev_ino_buf, 4, st.st_dev);
-		SIVAL64(num_dev_ino_buf, 4+8, st.st_ino);
 		send_msg(MSG_SUCCESS, num_dev_ino_buf, sizeof num_dev_ino_buf, -1);
 	} else
 		send_msg_int(MSG_SUCCESS, num);
@@ -1305,6 +1311,8 @@ static void unbackslash_arg(char *s)
 	*t = '\0';
 }
 
+#define MAX_DAEMON_ARGS (MAX_ARGS * 16)
+
 void read_args(int f_in, char *mod_name, char *buf, size_t bufsiz, int rl_nulls,
 	       int unescape, char ***argv_p, int *argc_p, char **request_p)
 {
@@ -1328,6 +1336,11 @@ void read_args(int f_in, char *mod_name, char *buf, size_t bufsiz, int rl_nulls,
 		if (read_line(f_in, buf, bufsiz, rl_flags) == 0)
 			break;
 
+		if (mod_name && argc >= MAX_DAEMON_ARGS - 1) {
+			rprintf(FERROR, "too many daemon arguments\n");
+			exit_cleanup(RERR_PROTOCOL);
+		}
+
 		if (argc == maxargs-1) {
 			maxargs += MAX_ARGS;
 			argv = realloc_array(argv, char *, maxargs);
@@ -1358,6 +1371,13 @@ void read_args(int f_in, char *mod_name, char *buf, size_t bufsiz, int rl_nulls,
 				dot_pos = argc;
 		}
 	}
+	/* glob_expand()/glob_match() reserve glob.argc+1 slots -- room for the
+	 * entry being added but not for this trailing NULL.  A post-dot line
+	 * whose " mod/" splits land argc on exactly maxargs (or any later
+	 * ENSURE_MEMSPACE doubling boundary) would otherwise make the next
+	 * store an 8-byte NULL write one slot past the argv allocation. */
+	if (argc >= maxargs)
+		argv = realloc_array(argv, char *, maxargs = argc + 1);
 	argv[argc] = NULL;
 
 	glob_expand(NULL, NULL, NULL, NULL);
@@ -1543,10 +1563,10 @@ static void read_a_msg(void)
 		if (msg_bytes != 4)
 			goto invalid_msg;
 		val = raw_read_int();
-		iobuf.in_multiplexed = 1;
 		io_error |= val;
 		if (am_receiver)
 			send_msg_int(MSG_IO_ERROR, val);
+		iobuf.in_multiplexed = 1;
 		break;
 	case MSG_IO_TIMEOUT:
 		if (msg_bytes != 4 || am_server || am_generator)
@@ -1563,17 +1583,17 @@ static void read_a_msg(void)
 		/* Support protocol-30 keep-alive method. */
 		if (msg_bytes != 0)
 			goto invalid_msg;
-		iobuf.in_multiplexed = 1;
 		if (am_sender)
 			maybe_send_keepalive(time(NULL), MSK_ALLOW_FLUSH);
+		iobuf.in_multiplexed = 1;
 		break;
 	case MSG_DELETED:
 		if (msg_bytes >= sizeof data)
 			goto overflow;
 		if (am_generator) {
 			raw_read_buf(data, msg_bytes);
-			iobuf.in_multiplexed = 1;
 			send_msg(MSG_DELETED, data, msg_bytes, 1);
+			iobuf.in_multiplexed = 1;
 			break;
 		}
 #ifdef ICONV_OPTION
@@ -1611,7 +1631,6 @@ static void read_a_msg(void)
 		} else
 #endif
 			raw_read_buf(data, msg_bytes);
-		iobuf.in_multiplexed = 1;
 		/* A directory name was sent with the trailing null */
 		if (msg_bytes > 0 && !data[msg_bytes-1])
 			log_delete(data, S_IFDIR);
@@ -1619,6 +1638,7 @@ static void read_a_msg(void)
 			data[msg_bytes] = '\0';
 			log_delete(data, S_IFREG);
 		}
+		iobuf.in_multiplexed = 1;
 		break;
 	case MSG_SUCCESS:
 		if (msg_bytes != (local_server ? 4+8+8 : 4)) {
@@ -1640,11 +1660,11 @@ static void read_a_msg(void)
 		if (msg_bytes != 4)
 			goto invalid_msg;
 		val = raw_read_int();
-		iobuf.in_multiplexed = 1;
 		if (am_generator)
 			got_flist_entry_status(FES_NO_SEND, val);
 		else
 			send_msg_int(MSG_NO_SEND, val);
+		iobuf.in_multiplexed = 1;
 		break;
 	case MSG_ERROR_SOCKET:
 	case MSG_ERROR_UTF8:
@@ -2052,6 +2072,21 @@ void read_sum_head(int f, struct sum_struct *sum)
 			(long)sum->blength, who_am_i());
 		exit_cleanup(RERR_PROTOCOL);
 	}
+	if (sum->count && sum->blength == 0) {
+		rprintf(FERROR, "Invalid zero block length [%s]\n",
+			who_am_i());
+		exit_cleanup(RERR_PROTOCOL);
+	}
+#if SIZEOF_CAPITAL_OFF_T < 8
+	/* The append-mode callers compute (OFF_T)count * blength; on a 32-bit
+	 * OFF_T that product can wrap even though both factors are individually
+	 * in range, corrupting the lseek/loop bounds.  Reject it early. */
+	if (sum->blength > 0 && sum->count > MAX_INT32 / sum->blength) {
+		rprintf(FERROR, "checksum count*blength overflows OFF_T [%s]\n",
+			who_am_i());
+		exit_cleanup(RERR_PROTOCOL);
+	}
+#endif
 	sum->s2length = protocol_version < 27 ? csum_length : (int)read_int(f);
 	if (sum->s2length < 0 || sum->s2length > xfer_sum_len) {
 		rprintf(FERROR, "Invalid checksum length %d [%s]\n",
@@ -2366,6 +2401,7 @@ int32 read_ndx(int f)
 {
 	static int32 prev_positive = -1, prev_negative = 1;
 	int32 *prev_ptr, num;
+	uint32 unum;
 	char b[4];
 
 	if (protocol_version < 30)
@@ -2385,11 +2421,20 @@ int32 read_ndx(int f)
 			b[3] = CVAL(b, 0) & ~0x80;
 			b[0] = b[1];
 			read_buf(f, b+1, 2);
-			num = IVAL(b, 0);
+			unum = IVAL(b, 0);
 		} else
-			num = (UVAL(b,0)<<8) + UVAL(b,1) + *prev_ptr;
+			unum = (UVAL(b,0)<<8) + UVAL(b,1) + (uint32)*prev_ptr;
 	} else
-		num = UVAL(b, 0) + *prev_ptr;
+		unum = UVAL(b, 0) + (uint32)*prev_ptr;
+	/* A peer-supplied index that overflows a signed int32 (used unchecked as a
+	 * file-list index) is a protocol violation -- reject it here rather than
+	 * relying on every downstream consumer to bounds-check. */
+	if (unum > (uint32)MAX_INT32) {
+		rprintf(FERROR, "Invalid file index: %lu [%s]\n",
+			(unsigned long)unum, who_am_i());
+		exit_cleanup(RERR_PROTOCOL);
+	}
+	num = (int32)unum;
 	*prev_ptr = num;
 	if (prev_ptr == &prev_negative)
 		num = -num;

lib/pool_alloc.c

@@ -44,6 +44,32 @@ struct align_test {
 #define PTR_ADD(b,o)	( (void*) ((char*)(b) + (o)) )
 #define PTR_SUB(b,o)	( (void*) ((char*)(b) - (o)) )
 
+/* Under AddressSanitizer, fence each pool_alloc() chunk with a poisoned
+ * redzone just below it (allocations grow downward from the top of an extent).
+ * A bump allocator hands out chunks from one big malloc, so ASan cannot see a
+ * write that underflows one chunk into its neighbour -- e.g. a miscomputed
+ * F_SUM() reaching before a file_struct's extras.  The redzone turns that into
+ * a hard ASan report.  We unpoison a whole extent whenever its space is reused
+ * (reset/reclaim), so legitimate later allocations never trip over old
+ * redzones; ASan unpoisons freed extents itself via free(). */
+#if defined(__SANITIZE_ADDRESS__)
+# define POOL_ASAN 1
+#elif defined(__has_feature)
+# if __has_feature(address_sanitizer)
+#  define POOL_ASAN 1
+# endif
+#endif
+
+#ifdef POOL_ASAN
+# include <sanitizer/asan_interface.h>
+# define POOL_REDZONE 16  /* >= the largest pool-relative underflow we guard */
+# define POOL_POISON(p,n)   ASAN_POISON_MEMORY_REGION((p), (n))
+# define POOL_UNPOISON(p,n) ASAN_UNPOISON_MEMORY_REGION((p), (n))
+#else
+# define POOL_POISON(p,n)   ((void)0)
+# define POOL_UNPOISON(p,n) ((void)0)
+#endif
+
 alloc_pool_t
 pool_create(size_t size, size_t quantum, void (*bomb)(const char*, const char*, int), int flags)
 {
@@ -165,7 +191,18 @@ pool_alloc(alloc_pool_t p, size_t len, const char *bomb_msg)
 
 	pool->extents->free -= len;
 
-	return PTR_ADD(pool->extents->start, pool->extents->free);
+	{
+		void *ret = PTR_ADD(pool->extents->start, pool->extents->free);
+#ifdef POOL_ASAN
+		size_t rz = pool->extents->free < POOL_REDZONE
+			  ? pool->extents->free : POOL_REDZONE;
+		if (rz) {
+			pool->extents->free -= rz;
+			POOL_POISON(PTR_ADD(pool->extents->start, pool->extents->free), rz);
+		}
+#endif
+		return ret;
+	}
 
   bomb_out:
 	if (pool->bomb)
@@ -215,6 +252,10 @@ pool_free(alloc_pool_t p, size_t len, void *addr)
 	if (!cur)
 		return;
 
+	/* This extent's space may be reused (and POOL_CLEAR may memset it)
+	 * below, so drop any redzones in it first. */
+	POOL_UNPOISON(cur->start, pool->size);
+
 	if (!prev) {
 		/* The "live" extent is kept ready for more allocations. */
 		if (cur->free + cur->bound + len >= pool->size) {
@@ -272,6 +313,8 @@ pool_free_old(alloc_pool_t p, void *addr)
 	if (!cur)
 		return;
 
+	POOL_UNPOISON(cur->start, pool->size);
+
 	if (addr == PTR_ADD(cur->start, cur->free)) {
 		if (prev) {
 			prev->next = NULL;

log.c

@@ -154,7 +154,15 @@ static void syslog_init()
 static void logfile_open(void)
 {
 	mode_t old_umask = umask(022 | orig_umask);
-	logfile_fp = fopen(logfile_name, "a");
+	/* --log-file/`log file =` are operator-supplied paths that may transit
+	 * attacker-writable dirs; a planted symlink could redirect root's log
+	 * into e.g. /root/.ssh/authorized_keys.  Refuse symlinks not owned by
+	 * uid 0 or our euid. */
+	int fd = open_no_attacker_symlinks(logfile_name,
+						O_WRONLY | O_APPEND | O_CREAT, 0644);
+	logfile_fp = fd >= 0 ? fdopen(fd, "a") : NULL;
+	if (!logfile_fp && fd >= 0)
+		close(fd);
 	umask(old_umask);
 	if (!logfile_fp) {
 		int fopen_errno = errno;
@@ -273,8 +281,12 @@ void rwrite(enum logcode code, const char *buf, int len, int is_utf8)
 		if (am_daemon > 0 && code != FCLIENT)
 			code = FLOG;
 	} else if (send_msgs_to_gen) {
-		assert(!is_utf8);
-		/* Pass the message to our sibling in native charset. */
+		/* Pass the message to our sibling in native charset.  is_utf8
+		 * may be set here if a malicious peer sends MSG_INFO/MSG_ERROR
+		 * to a daemon receiver (read_a_msg passes !am_generator); the
+		 * old assert(!is_utf8) made that a remotely-reachable abort.
+		 * Forwarding the bytes raw is safe -- the generator's rwrite()
+		 * gets is_utf8=0 and filtered_fwrite escapes non-printables. */
 		send_msg((enum msgcode)code, buf, len, 0);
 		return;
 	}
@@ -691,7 +703,7 @@ static void log_formatted(enum logcode code, const char *format, const char *op,
 		case 'C':
 			n = NULL;
 			if (S_ISREG(file->mode)) {
-				if (always_checksum)
+				if (always_checksum && !(iflags & ITEM_DELETED))
 					n = sum_as_hex(file_sum_nni->num, F_SUM(file), 1);
 				else if (iflags & ITEM_TRANSFER)
 					n = sum_as_hex(xfer_sum_nni->num, sender_file_sum, 0);

match.c

@@ -325,6 +325,7 @@ static void hash_search(int f,struct sum_struct *s,
 			 && (!updating_basis_file || s->sums[want_i].offset >= offset
 			  || s->sums[want_i].flags & SUMFLG_SAME_OFFSET)
 			 && sum == s->sums[want_i].sum1
+			 && l == s->sums[want_i].len
 			 && memcmp(sum2, sum2_at(s, want_i), s->s2length) == 0) {
 				/* we've found an adjacent match - the RLL coder
 				 * will be happy */
@@ -402,6 +403,14 @@ void match_sums(int f, struct sum_struct *s, struct map_struct *buf, OFF_T len)
 	sum_init(xfer_sum_nni, checksum_seed);
 
 	if (append_mode > 0) {
+		if (s->flength > len) {
+			/* A hostile or confused peer can claim a verified-prefix
+			 * length that exceeds what we have on disk -- including
+			 * for an empty local file, where buf is NULL and the
+			 * map_ptr() calls below would dereference it.  Clamp to
+			 * what we can actually read. */
+			s->flength = len;
+		}
 		if (append_mode == 2) {
 			OFF_T j = 0;
 			for (j = CHUNK_SIZE; j < s->flength; j += CHUNK_SIZE) {

rsync.h

@@ -708,6 +708,8 @@ typedef unsigned int size_t;
 # define SIZEOF_INT64 SIZEOF_OFF_T
 #endif
 
+#define MAX_INT32 ((int32)0x7fffffff)
+
 #define HT_KEY32 0
 #define HT_KEY64 1