update NEWS for 3.4.0

make it easier to spot unpatched servers

NEWS.md

@@ -1,10 +1,30 @@
-# NEWS for rsync 3.3.1 (UNRELEASED)
+# NEWS for rsync 3.4.0 (14th Jan 2025)
+
+Release 3.4.0 is a security release that fixes a number of important vulnerabilities.
+
+Many thanks to Simon Scannell, Pedro Gallegos, and Jasiel Spelman at
+Google Cloud Vulnerability Research and Aleksei Gorban (Loqpa) for
+discovering these vulnerabilities and working with the rsync project
+to develop and test fixes.
+
+For more details on the vulnerabilities please see the CERT report
+https://kb.cert.org/vuls/id/952657
 
 ## Changes in this version:
 
 ### SECURITY FIXES:
 
-- Fixed a buffer overflow when the sum2 digest algorithm is SHA1.
+- CVE-2024-12084 - Heap Buffer Overflow in Checksum Parsing.
+
+- CVE-2024-12085 - Info Leak via uninitialized Stack contents defeats ASLR.
+  
+- CVE-2024-12086 - Server leaks arbitrary client files.
+
+- CVE-2024-12087 - Server can make client write files outside of destination directory using symbolic links.
+
+- CVE-2024-12088 - --safe-links Bypass.
+
+- CVE-2024-12747 - symlink race condition.
 
 ### BUG FIXES:
 
@@ -12,6 +32,8 @@
 
 - Fixed an incorrect extern variable's type that caused an ACL issue on macOS.
 
+- Fixed IPv6 configure check
+
 ### INTERNAL:
 
 - Updated included popt to version 1.19.
@@ -22,8 +44,9 @@
 
 - Improved packaging/var-checker to identify variable type issues.
 
-------------------------------------------------------------------------------
+- added FreeBSD and Solaris CI builds
 
+------------------------------------------------------------------------------
 # NEWS for rsync 3.3.0 (6 Apr 2024)
 
 ## Changes in this version:

checksum.c

@@ -406,7 +406,7 @@ void file_checksum(const char *fname, const STRUCT_STAT *st_p, char *sum)
 	int32 remainder;
 	int fd;
 
-	fd = do_open(fname, O_RDONLY, 0);
+	fd = do_open_checklinks(fname);
 	if (fd == -1) {
 		memset(sum, 0, file_sum_len);
 		return;

flist.c

@@ -1390,7 +1390,7 @@ struct file_struct *make_file(const char *fname, struct file_list *flist,
 
 	if (copy_devices && am_sender && IS_DEVICE(st.st_mode)) {
 		if (st.st_size == 0) {
-			int fd = do_open(fname, O_RDONLY, 0);
+			int fd = do_open_checklinks(fname);
 			if (fd >= 0) {
 				st.st_size = get_device_size(fd, fname);
 				close(fd);
@@ -2584,6 +2584,19 @@ struct file_list *recv_file_list(int f, int dir_ndx)
 		init_hard_links();
 #endif
 
+	if (inc_recurse && dir_ndx >= 0) {
+		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 (file->flags & FLAG_GOT_DIR_FLIST) {
+			rprintf(FERROR_XFER, "rsync: refusing malicious duplicate flist for dir %d\n", dir_ndx);
+			exit_cleanup(RERR_PROTOCOL);
+		}
+		file->flags |= FLAG_GOT_DIR_FLIST;
+	}
+
 	flist = flist_new(0, "recv_file_list");
 	flist_expand(flist, FLIST_START_LARGE);
 

generator.c

@@ -1798,7 +1798,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 
 	if (write_devices && IS_DEVICE(sx.st.st_mode) && sx.st.st_size == 0) {
 		/* This early open into fd skips the regular open below. */
-		if ((fd = do_open(fnamecmp, O_RDONLY, 0)) >= 0)
+		if ((fd = do_open_nofollow(fnamecmp, O_RDONLY)) >= 0)
 			real_sx.st.st_size = sx.st.st_size = get_device_size(fd, fnamecmp);
 	}
 
@@ -1867,7 +1867,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 	}
 
 	/* open the file */
-	if (fd < 0 && (fd = do_open(fnamecmp, O_RDONLY, 0)) < 0) {
+	if (fd < 0 && (fd = do_open_checklinks(fnamecmp)) < 0) {
 		rsyserr(FERROR, errno, "failed to open %s, continuing",
 			full_fname(fnamecmp));
 	  pretend_missing:

match.c

@@ -147,6 +147,9 @@ static void hash_search(int f,struct sum_struct *s,
 	int more;
 	schar *map;
 
+	// prevent possible memory leaks
+	memset(sum2, 0, sizeof sum2);
+
 	/* want_i is used to encourage adjacent matches, allowing the RLL
 	 * coding of the output to work more efficiently. */
 	want_i = 0;

receiver.c

@@ -66,6 +66,7 @@ extern char sender_file_sum[MAX_DIGEST_LEN];
 extern struct file_list *cur_flist, *first_flist, *dir_flist;
 extern filter_rule_list daemon_filter_list;
 extern OFF_T preallocated_len;
+extern int fuzzy_basis;
 
 extern struct name_num_item *xfer_sum_nni;
 extern int xfer_sum_len;
@@ -551,6 +552,8 @@ int recv_files(int f_in, int f_out, char *local_name)
 	progress_init();
 
 	while (1) {
+		const char *basedir = NULL;
+
 		cleanup_disable();
 
 		/* This call also sets cur_flist. */
@@ -716,28 +719,34 @@ int recv_files(int f_in, int f_out, char *local_name)
 				fnamecmp = get_backup_name(fname);
 				break;
 			case FNAMECMP_FUZZY:
+				if (fuzzy_basis == 0) {
+					rprintf(FERROR_XFER, "rsync: refusing malicious fuzzy operation for %s\n", xname);
+					exit_cleanup(RERR_PROTOCOL);
+				}
 				if (file->dirname) {
-					pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, file->dirname, xname);
-					fnamecmp = fnamecmpbuf;
-				} else
-					fnamecmp = xname;
+					basedir = file->dirname;
+				}
+				fnamecmp = xname;
 				break;
 			default:
 				if (fnamecmp_type > FNAMECMP_FUZZY && fnamecmp_type-FNAMECMP_FUZZY <= basis_dir_cnt) {
 					fnamecmp_type -= FNAMECMP_FUZZY + 1;
 					if (file->dirname) {
-						stringjoin(fnamecmpbuf, sizeof fnamecmpbuf,
-							   basis_dir[fnamecmp_type], "/", file->dirname, "/", xname, NULL);
-					} else
-						pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, basis_dir[fnamecmp_type], xname);
+						pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, basis_dir[fnamecmp_type], file->dirname);
+						basedir = fnamecmpbuf;
+					} else {
+						basedir = basis_dir[fnamecmp_type];
+					}
+					fnamecmp = xname;
 				} else if (fnamecmp_type >= basis_dir_cnt) {
 					rprintf(FERROR,
 						"invalid basis_dir index: %d.\n",
 						fnamecmp_type);
 					exit_cleanup(RERR_PROTOCOL);
-				} else
-					pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, basis_dir[fnamecmp_type], fname);
-				fnamecmp = fnamecmpbuf;
+				} else {
+					basedir = basis_dir[fnamecmp_type];
+					fnamecmp = fname;
+				}
 				break;
 			}
 			if (!fnamecmp || (daemon_filter_list.head
@@ -760,25 +769,31 @@ int recv_files(int f_in, int f_out, char *local_name)
 		}
 
 		/* open the file */
-		fd1 = do_open(fnamecmp, O_RDONLY, 0);
+		fd1 = secure_relative_open(basedir, fnamecmp, O_RDONLY, 0);
 
 		if (fd1 == -1 && protocol_version < 29) {
 			if (fnamecmp != fname) {
 				fnamecmp = fname;
 				fnamecmp_type = FNAMECMP_FNAME;
-				fd1 = do_open(fnamecmp, O_RDONLY, 0);
+				fd1 = do_open_nofollow(fnamecmp, O_RDONLY);
 			}
 
 			if (fd1 == -1 && basis_dir[0]) {
 				/* pre-29 allowed only one alternate basis */
-				pathjoin(fnamecmpbuf, sizeof fnamecmpbuf,
-					 basis_dir[0], fname);
-				fnamecmp = fnamecmpbuf;
+				basedir = basis_dir[0];
+				fnamecmp = fname;
 				fnamecmp_type = FNAMECMP_BASIS_DIR_LOW;
-				fd1 = do_open(fnamecmp, O_RDONLY, 0);
+				fd1 = secure_relative_open(basedir, fnamecmp, O_RDONLY, 0);
 			}
 		}
 
+		if (basedir) {
+			// for the following code we need the full
+			// path name as a single string
+			pathjoin(fnamecmpbuf, sizeof fnamecmpbuf, basedir, fnamecmp);
+			fnamecmp = fnamecmpbuf;
+		}
+
 		one_inplace = inplace_partial && fnamecmp_type == FNAMECMP_PARTIAL_DIR;
 		updating_basis_or_equiv = one_inplace
 		    || (inplace && (fnamecmp == fname || fnamecmp_type == FNAMECMP_BACKUP));

rsync.h

@@ -84,6 +84,7 @@
 #define FLAG_DUPLICATE (1<<4)	/* sender */
 #define FLAG_MISSING_DIR (1<<4)	/* generator */
 #define FLAG_HLINKED (1<<5)	/* receiver/generator (checked on all types) */
+#define FLAG_GOT_DIR_FLIST (1<<5)/* sender/receiver/generator - dir_flist only */
 #define FLAG_HLINK_FIRST (1<<6)	/* receiver/generator (w/FLAG_HLINKED) */
 #define FLAG_IMPLIED_DIR (1<<6)	/* sender/receiver/generator (dirs only) */
 #define FLAG_HLINK_LAST (1<<7)	/* receiver/generator */
@@ -110,7 +111,7 @@
 
 /* Update this if you make incompatible changes and ALSO update the
  * SUBPROTOCOL_VERSION if it is not a final (official) release. */
-#define PROTOCOL_VERSION 31
+#define PROTOCOL_VERSION 32
 
 /* This is used when working on a new protocol version or for any unofficial
  * protocol tweaks.  It should be a non-zero value for each pre-release repo

sender.c

@@ -350,7 +350,7 @@ void send_files(int f_in, int f_out)
 			exit_cleanup(RERR_PROTOCOL);
 		}
 
-		fd = do_open(fname, O_RDONLY, 0);
+		fd = do_open_checklinks(fname);
 		if (fd == -1) {
 			if (errno == ENOENT) {
 				enum logcode c = am_daemon && protocol_version < 28 ? FERROR : FWARNING;

syscall.c

@@ -33,6 +33,8 @@
 #include <sys/syscall.h>
 #endif
 
+#include "ifuncs.h"
+
 extern int dry_run;
 extern int am_root;
 extern int am_sender;
@@ -43,6 +45,8 @@ extern int preallocate_files;
 extern int preserve_perms;
 extern int preserve_executability;
 extern int open_noatime;
+extern int copy_links;
+extern int copy_unsafe_links;
 
 #ifndef S_BLKSIZE
 # if defined hpux || defined __hpux__ || defined __hpux
@@ -707,3 +711,100 @@ int do_open_nofollow(const char *pathname, int flags)
 
 	return fd;
 }
+
+/*
+  open a file relative to a base directory. The basedir can be NULL,
+  in which case the current working directory is used. The relpath
+  must be a relative path, and the relpath must not contain any
+  elements in the path which follow symlinks (ie. like O_NOFOLLOW, but
+  applies to all path components, not just the last component)
+
+  The relpath must also not contain any ../ elements in the path
+*/
+int secure_relative_open(const char *basedir, const char *relpath, int flags, mode_t mode)
+{
+	if (!relpath || relpath[0] == '/') {
+		// must be a relative path
+		errno = EINVAL;
+		return -1;
+	}
+	if (strncmp(relpath, "../", 3) == 0 || strstr(relpath, "/../")) {
+		// no ../ elements allowed in the relpath
+		errno = EINVAL;
+		return -1;
+	}
+
+#if !defined(O_NOFOLLOW) || !defined(O_DIRECTORY)
+	// really old system, all we can do is live with the risks
+	if (!basedir) {
+		return open(relpath, flags, mode);
+	}
+	char fullpath[MAXPATHLEN];
+	pathjoin(fullpath, sizeof fullpath, basedir, relpath);
+	return open(fullpath, flags, mode);
+#else
+	int dirfd = AT_FDCWD;
+	if (basedir != NULL) {
+		dirfd = openat(AT_FDCWD, basedir, O_RDONLY | O_DIRECTORY);
+		if (dirfd == -1) {
+			return -1;
+		}
+	}
+	int retfd = -1;
+
+	char *path_copy = my_strdup(relpath, __FILE__, __LINE__);
+	if (!path_copy) {
+		return -1;
+	}
+	
+	for (const char *part = strtok(path_copy, "/");
+	     part != NULL;
+	     part = strtok(NULL, "/"))
+	{
+		int next_fd = openat(dirfd, part, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+		if (next_fd == -1 && errno == ENOTDIR) {
+			if (strtok(NULL, "/") != NULL) {
+				// this is not the last component of the path
+				errno = ELOOP;
+				goto cleanup;
+			}
+			// this could be the last component of the path, try as a file
+			retfd = openat(dirfd, part, flags | O_NOFOLLOW, mode);
+			goto cleanup;
+		}
+		if (next_fd == -1) {
+			goto cleanup;
+		}
+		if (dirfd != AT_FDCWD) close(dirfd);
+		dirfd = next_fd;
+	}
+
+	// the path must be a directory
+	errno = EINVAL;
+
+cleanup:
+	free(path_copy);
+	if (dirfd != AT_FDCWD) {
+		close(dirfd);
+	}
+	return retfd;
+#endif // O_NOFOLLOW, O_DIRECTORY
+}
+
+/*
+  varient of do_open/do_open_nofollow which does do_open() if the
+  copy_links or copy_unsafe_links options are set and does
+  do_open_nofollow() otherwise
+
+  This is used to prevent a race condition where an attacker could be
+  switching a file between being a symlink and being a normal file
+
+  The open is always done with O_RDONLY flags
+ */
+int do_open_checklinks(const char *pathname)
+{
+	if (copy_links || copy_unsafe_links) {
+		return do_open(pathname, O_RDONLY, 0);
+	}
+	return do_open_nofollow(pathname, O_RDONLY);
+}

t_unsafe.c

@@ -28,6 +28,9 @@ int am_root = 0;
 int am_sender = 1;
 int read_only = 0;
 int list_only = 0;
+int copy_links = 0;
+int copy_unsafe_links = 0;
+
 short info_levels[COUNT_INFO], debug_levels[COUNT_DEBUG];
 
 int

testsuite/safe-links.test

@@ -0,0 +1,55 @@
+#!/bin/sh
+
+. "$suitedir/rsync.fns"
+
+test_symlink() {
+	is_a_link "$1" || test_fail "File $1 is not a symlink"
+}
+
+test_regular() {
+	if [ ! -f "$1" ]; then
+		test_fail "File $1 is not regular file or not exists"
+	fi
+}
+
+test_notexist() {
+        if [ -e "$1" ]; then
+                test_fail "File $1 exists"
+	fi
+        if [ -h "$1" ]; then
+                test_fail "File $1 exists as a symlink"
+	fi
+}
+
+cd "$tmpdir"
+
+mkdir from
+
+mkdir "from/safe"
+mkdir "from/unsafe"
+
+mkdir "from/safe/files"
+mkdir "from/safe/links"
+
+touch "from/safe/files/file1"
+touch "from/safe/files/file2"
+touch "from/unsafe/unsafefile"
+
+ln -s ../files/file1 "from/safe/links/"
+ln -s ../files/file2 "from/safe/links/"
+ln -s ../../unsafe/unsafefile "from/safe/links/"
+ln -s a/a/a/../../../unsafe2 "from/safe/links/"
+
+#echo "LISTING FROM"
+#ls -lR from
+
+echo "rsync with relative path and just -a"
+$RSYNC -avv --safe-links from/safe/ to
+
+#echo "LISTING TO"
+#ls -lR to
+
+test_symlink to/links/file1
+test_symlink to/links/file2
+test_notexist to/links/unsafefile
+test_notexist to/links/unsafe2

testsuite/unsafe-byname.test

@@ -40,7 +40,7 @@ test_unsafe ..//../dest 		from/dir			unsafe
 test_unsafe ..				from/file			safe
 test_unsafe ../..			from/file			unsafe
 test_unsafe ..//..			from//file			unsafe
-test_unsafe dir/..			from				safe
+test_unsafe dir/..			from				unsafe
 test_unsafe dir/../..			from				unsafe
 test_unsafe dir/..//..			from				unsafe
 

tls.c

@@ -49,6 +49,9 @@ int list_only = 0;
 int link_times = 0;
 int link_owner = 0;
 int nsec_times = 0;
+int safe_symlinks = 0;
+int copy_links = 0;
+int copy_unsafe_links = 0;
 
 #ifdef SUPPORT_XATTRS
 

trimslash.c

@@ -26,6 +26,8 @@ int am_root = 0;
 int am_sender = 1;
 int read_only = 1;
 int list_only = 0;
+int copy_links = 0;
+int copy_unsafe_links = 0;
 
 int
 main(int argc, char **argv)

util1.c

@@ -365,7 +365,7 @@ int copy_file(const char *source, const char *dest, int tmpfilefd, mode_t mode)
 	int len;   /* Number of bytes read into `buf'. */
 	OFF_T prealloc_len = 0, offset = 0;
 
-	if ((ifd = do_open(source, O_RDONLY, 0)) < 0) {
+	if ((ifd = do_open_nofollow(source, O_RDONLY)) < 0) {
 		int save_errno = errno;
 		rsyserr(FERROR_XFER, errno, "open %s", full_fname(source));
 		errno = save_errno;
@@ -1318,7 +1318,14 @@ int handle_partial_dir(const char *fname, int create)
  *
  * "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. */
+ * (including its name), as referenced from the root of the transfer.
+ *
+ * NOTE: this also rejects dest names with a .. component in other
+ * than the first component of the name ie. it rejects names such as
+ * a/b/../x/y. This needs to be done as the leading subpaths 'a' or
+ * 'b' could later be replaced with symlinks such as a link to '.'
+ * resulting in the link being transferred now becoming unsafe
+ */
 int unsafe_symlink(const char *dest, const char *src)
 {
 	const char *name, *slash;
@@ -1328,6 +1335,23 @@ int unsafe_symlink(const char *dest, const char *src)
 	if (!dest || !*dest || *dest == '/')
 		return 1;
 
+	// reject destinations with /../ in the name other than at the start of the name
+	const char *dest2 = dest;
+	while (strncmp(dest2, "../", 3) == 0) {
+	    dest2 += 3;
+	    while (*dest2 == '/') {
+		// allow for ..//..///../foo
+		dest2++;
+	    }
+	}
+	if (strstr(dest2, "/../"))
+	    return 1;
+
+	// reject if the destination ends in /..
+	const size_t dlen = strlen(dest);
+	if (dlen > 3 && strcmp(&dest[dlen-3], "/..") == 0)
+	    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. */

version.h

@@ -1,2 +1,2 @@
-#define RSYNC_VERSION "3.3.1dev"
+#define RSYNC_VERSION "3.4.0"
 #define MAINTAINER_TZ_OFFSET -7.0