flist/xattrs/generator/uidlist/clientserver: plug audit-reported leaks

Five error-path/cleanup memory leaks found by an external audit:

- flist.c send_file_name: free the ACL loaded by get_acl() when a later
  get_xattr() fails (and on the get_acl error path).
- xattrs.c copy_xattrs: free the xattr datum buffer when the setxattr fails.
- generator.c recv_generator: free real_sx at the cleanup label (the
  directory branch loaded its ACL via set_file_attrs but only the
  regular-file path freed it); zero-init real_sx so the early gotos are safe.
- uidlist.c send_one_list: free the strdup'd id-0 name after send_one_name.
- clientserver.c start_inband_exchange: free modname on the early error
  returns (it was freed only on the success path).

ASan/LSan regression tests cover the generator, uidlist and clientserver
leaks; the flist and xattrs leaks need a forced syscall failure and are
covered by the audit's standalone harnesses.

Reported-by: Leonid Bugaev <leonsbox@gmail.com>
(cherry picked from commit 078f3b99f4004510d418ee9d97d9b775dc8587bc)

clientserver.c

@@ -275,8 +275,10 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 	if (!user)
 		user = getenv("LOGNAME");
 
-	if (exchange_protocols(f_in, f_out, line, sizeof line, 1) < 0)
+	if (exchange_protocols(f_in, f_out, line, sizeof line, 1) < 0) {
+		free(modname);
 		return -1;
+	}
 
 	if (early_input_file) {
 		STRUCT_STAT st;
@@ -289,12 +291,14 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 			rsyserr(FERROR, errno, "failed to open %s", early_input_file);
 			if (f)
 				fclose(f);
+			free(modname);
 			return -1;
 		}
 		early_input_len = st.st_size;
 		if (early_input_len > (int)sizeof line) {
 			rprintf(FERROR, "%s is > %d bytes.\n", early_input_file, (int)sizeof line);
 			fclose(f);
+			free(modname);
 			return -1;
 		}
 		if (early_input_len > 0) {
@@ -304,6 +308,7 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 				if (feof(f)) {
 					rprintf(FERROR, "Early EOF in %s\n", early_input_file);
 					fclose(f);
+					free(modname);
 					return -1;
 				}
 				len = fread(line, 1, early_input_len, f);
@@ -380,6 +385,7 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 	while (1) {
 		if (!read_line_old(f_in, line, sizeof line, 0)) {
 			rprintf(FERROR, "rsync: didn't get server startup line\n");
+			free(modname);
 			return -1;
 		}
 
@@ -403,6 +409,7 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 			rprintf(FERROR, "%s\n", line);
 			/* This is always fatal; the server will now
 			 * close the socket. */
+			free(modname);
 			return -1;
 		}
 

flist.c

@@ -1794,6 +1794,7 @@ static struct file_struct *send_file_name(int f, struct file_list *flist,
 			sx.st.st_mode = file->mode;
 			if (get_acl(fname, &sx) < 0) {
 				io_error |= IOERR_GENERAL;
+				free_acl(&sx);
 				return NULL;
 			}
 		}
@@ -1803,6 +1804,9 @@ static struct file_struct *send_file_name(int f, struct file_list *flist,
 			sx.st.st_mode = file->mode;
 			if (get_xattr(fname, &sx) < 0) {
 				io_error |= IOERR_GENERAL;
+#ifdef SUPPORT_ACLS
+				free_acl(&sx); /* get_acl() above may have loaded one */
+#endif
 				return NULL;
 			}
 		}

generator.c

@@ -1441,7 +1441,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 	static int need_fuzzy_dirlist = 0;
 	struct file_struct *fuzzy_file = NULL;
 	int fd = -1, f_copy = -1;
-	stat_x sx = {0}, real_sx;
+	stat_x sx = {0}, real_sx = {0};
 	STRUCT_STAT partial_st;
 	struct file_struct *back_file = NULL;
 	int statret, real_ret, stat_errno;
@@ -2250,6 +2250,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 	}
 
 	free_stat_x(&sx);
+	free_stat_x(&real_sx);
 }
 
 /* If we are replacing an existing hard link, symlink, device, or special file,

testsuite/inband-modname-leak_test.py

@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+"""KI-17: start_inband_exchange leaks modname on early error returns.
+
+clientserver.c:start_inband_exchange allocates `modname` (new_array) and frees
+it only on the success path; the early `return -1` error paths (e.g. the
+daemon replying @ERROR for an unknown module) leak it.  Client-side, one-shot
+per failed connection.
+
+ASan/LSan reproducer: connect to a daemon requesting a non-existent module so
+the @ERROR path is taken, then assert no client leak report names
+start_inband_exchange.  Gated on an AddressSanitizer build and --use-tcp.
+"""
+
+import glob
+import os
+import subprocess
+
+from rsyncfns import (
+    FROMDIR, SCRATCHDIR, RSYNC,
+    make_tree, require_asan, require_tcp, rmtree, rsync_argv,
+    start_test_daemon, test_fail,
+)
+
+DAEMON_PORT = 12896
+require_tcp("the daemon @ERROR handshake needs a real TCP peer")
+require_asan("KI-17 modname leak is only observable under AddressSanitizer/LSan", RSYNC)
+
+src = FROMDIR
+rmtree(src)
+make_tree(src, depth=1)
+
+conf = SCRATCHDIR / 'modname-leak.conf'
+conf.write_text(
+    f"pid file = {SCRATCHDIR}/rsyncd.pid\n"
+    "use chroot = no\n"
+    f"log file = {SCRATCHDIR}/rsyncd.log\n"
+    f"\n[realmod]\n\tpath = {src}\n\tread only = yes\n"
+)
+url = start_test_daemon(conf, DAEMON_PORT)
+
+asan_log = SCRATCHDIR / 'modname-leak-asan'
+for stale in glob.glob(f"{asan_log}.*"):
+    os.unlink(stale)
+os.environ['ASAN_OPTIONS'] = (
+    f"detect_leaks=1:abort_on_error=0:log_path={asan_log}"
+)
+
+# Request a module that does not exist: the daemon replies @ERROR and the
+# client's start_inband_exchange takes the early `return -1` that leaks modname.
+p = subprocess.run(rsync_argv('-r', f'{url}no-such-module/'),
+                   capture_output=True, text=True)
+
+# Non-vacuity: the connection must have been refused via the @ERROR path that
+# leaks modname (rc != 0 and an "Unknown module" / @ERROR diagnostic).
+if p.returncode == 0:
+    test_fail("connection to a non-existent module unexpectedly succeeded; "
+              "the modname-leak path was not exercised")
+if 'ERROR' not in p.stderr and 'nknown module' not in p.stderr:
+    test_fail(f"expected an @ERROR/unknown-module rejection; got:\n{p.stderr}")
+
+reports = ''.join(open(r, errors='replace').read()
+                  for r in glob.glob(f"{asan_log}.*"))
+if 'start_inband_exchange' in reports:
+    test_fail("start_inband_exchange leaked modname on the @ERROR path (KI-17):\n"
+              + reports[:1500])
+
+print("inband-modname-leak: start_inband_exchange does not leak modname")

testsuite/recv-generator-acl-leak_test.py

@@ -0,0 +1,68 @@
+#!/usr/bin/env python3
+"""KI-23: recv_generator leaks real_sx ACL data on the directory path.
+
+In generator.c:recv_generator the directory branch calls set_file_attrs() on
+&real_sx, which (under --acls) loads the destination's current ACL into
+real_sx via get_acl_fdat(), then does `goto cleanup`.  The cleanup label only
+frees sx, never real_sx (real_sx is freed solely on the regular-file path), so
+each pre-existing ACL-bearing destination directory leaks its ACL data.
+
+ASan/LSan reproducer: sync --acls onto a destination directory that already
+carries an ACL, then assert no leak report names recv_generator.  Gated on an
+AddressSanitizer build and on setfacl being available.
+"""
+
+import glob
+import os
+import shutil
+import subprocess
+
+from rsyncfns import (
+    SCRATCHDIR, RSYNC,
+    acls_supported, require_asan, rmtree, rsync_argv, test_fail, test_skipped,
+)
+
+require_asan("KI-23 real_sx ACL leak is only observable under AddressSanitizer/LSan", RSYNC)
+if not acls_supported():
+    test_skipped("rsync built without ACL support, or filesystem rejects ACLs")
+if not shutil.which('setfacl'):
+    test_skipped("setfacl not available to plant a destination-directory ACL")
+
+base = SCRATCHDIR / 'acl-leak'
+rmtree(base)
+src = base / 'src'
+dst = base / 'dst'
+(src / 'sub').mkdir(parents=True)
+(src / 'sub' / 'f.txt').write_text("updated-content\n")  # different size -> always transferred
+(dst / 'sub').mkdir(parents=True)
+(dst / 'sub' / 'f.txt').write_text("old\n")
+
+# Plant an ACL on the pre-existing destination directory so recv_generator's
+# directory branch loads it into real_sx (the leaked allocation).
+if subprocess.run(['setfacl', '-m', 'u:nobody:rwx', str(dst / 'sub')]).returncode != 0:
+    test_skipped("setfacl could not set an ACL on the destination directory")
+
+asan_log = base / 'acl-leak-asan'
+for stale in glob.glob(f"{asan_log}.*"):
+    os.unlink(stale)
+os.environ['ASAN_OPTIONS'] = (
+    f"detect_leaks=1:abort_on_error=0:log_path={asan_log}"
+)
+
+p = subprocess.run(rsync_argv('-a', '--acls', f'{src}/', f'{dst}/'),
+                   capture_output=True, text=True)
+
+# Non-vacuity: the transfer must have actually run through recv_generator's
+# directory branch (the dest dir pre-exists with an ACL, and its child file is
+# updated).  We can't check the exit code: under detect_leaks=1 rsync's
+# intentional at-exit leaks already force a nonzero status.
+if (dst / 'sub' / 'f.txt').read_text() != "updated-content\n":
+    test_fail(f"--acls transfer did not update the destination; leak path not exercised:\n{p.stderr}")
+
+reports = ''.join(open(r, errors='replace').read()
+                  for r in glob.glob(f"{asan_log}.*"))
+if 'recv_generator' in reports:
+    test_fail("recv_generator leaked real_sx ACL data on the directory path (KI-23):\n"
+              + reports[:1500])
+
+print("recv-generator-acl-leak: recv_generator does not leak real_sx ACL data")

testsuite/uidlist-id0-name-leak_test.py

@@ -0,0 +1,56 @@
+#!/usr/bin/env python3
+"""KI-25: send_one_list leaks the id-0 (root) name string.
+
+uidlist.c:send_one_list, when xmit_id0_names is negotiated, passes
+uid_to_user(0)/gid_to_group(0) straight to send_one_name(); those return a
+strdup'd name (uidlist.c:120/137) whose pointer is never stored or freed.  Up
+to two small strings leak per transfer.
+
+ASan/LSan reproducer: run an owner/group-preserving transfer (which sends the
+uid+gid lists with the id-0 name) and assert no leak report names send_one_list.
+Gated on an AddressSanitizer build.
+"""
+
+import glob
+import os
+import subprocess
+
+from rsyncfns import (
+    FROMDIR, SCRATCHDIR, RSYNC,
+    require_asan, rmtree, rsync_argv, test_fail,
+)
+
+require_asan("KI-25 id-0 name leak is only observable under AddressSanitizer/LSan", RSYNC)
+
+src = FROMDIR
+rmtree(src)
+src.mkdir(parents=True)
+(src / 'f.txt').write_text("hello\n")
+
+asan_log = SCRATCHDIR / 'id0-leak-asan'
+for stale in glob.glob(f"{asan_log}.*"):
+    os.unlink(stale)
+os.environ['ASAN_OPTIONS'] = (
+    f"detect_leaks=1:abort_on_error=0:log_path={asan_log}"
+)
+
+# -o/-g make the sender transmit the uid+gid lists; a modern<->modern transfer
+# negotiates xmit_id0_names, so send_one_list emits the id-0 name.  --list-only
+# makes the client exit via exit() (so LSan runs) rather than the _exit() of the
+# normal receiver/generator shutdown that would otherwise hide the leak.
+p = subprocess.run(rsync_argv('-og', '--list-only', f'{src}/'),
+                   capture_output=True, text=True)
+
+# Non-vacuity: the listing must have actually run (proving send_id_lists ->
+# send_one_list executed).  We can't check the exit code: under detect_leaks=1
+# rsync's intentional at-exit leaks already force a nonzero status.
+if 'f.txt' not in p.stdout:
+    test_fail(f"--list-only did not list the source file; leak path not exercised:\n{p.stdout}{p.stderr}")
+
+reports = ''.join(open(r, errors='replace').read()
+                  for r in glob.glob(f"{asan_log}.*"))
+if 'send_one_list' in reports:
+    test_fail("send_one_list leaked the id-0 name string (KI-25):\n"
+              + reports[:1500])
+
+print("uidlist-id0-name-leak: send_one_list does not leak the id-0 name")

uidlist.c

@@ -397,9 +397,11 @@ static void send_one_list(int f, struct idlist *idlist, int usernames)
 
 	/* Terminate the uid list with 0 (which was excluded above).
 	 * A modern rsync also sends the name of id 0. */
-	if (xmit_id0_names)
-		send_one_name(f, 0, usernames ? uid_to_user(0) : gid_to_group(0));
-	else
+	if (xmit_id0_names) {
+		const char *name = usernames ? uid_to_user(0) : gid_to_group(0);
+		send_one_name(f, 0, name);
+		free((char *)name);
+	} else
 		write_varint30(f, 0);
 }
 

xattrs.c

@@ -385,6 +385,7 @@ int copy_xattrs(const char *source, const char *dest, int dest_fd)
 				"copy_xattrs: %ssetxattr(%s,\"%s\") failed",
 				dest_fd >= 0 ? "f" : "l", full_fname(dest), name);
 			errno = save_errno;
+			free(ptr);
 			return -1;
 		}
 		free(ptr);