From 0a5fa00fdcbacbebb89daca0ae68ae320f22dc74 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 5 May 2026 16:48:16 +1000 Subject: [PATCH] receiver: add parent_ndx<0 guard, mirroring 797e17f Commit 797e17f ("fixed an invalid access to files array") added a parent_ndx < 0 guard to send_files() in sender.c, but the visually- identical block in recv_files() in receiver.c was not updated. A malicious rsync:// server can therefore drive any connecting client into the same out-of-bounds dir_flist->files[-1] read followed by a file_struct dereference in f_name() one line later. Reach: protocol-30+ default (inc_recurse) makes flist.c:2745 set parent_ndx = -1 on the first received flist when the sender omits a leading "." entry; rsync.c flist_for_ndx() does not reject ndx == 0 in that state because the range check evaluates 0 < 0 = false; and read_ndx_and_attrs() only validates ndx with the ITEM_TRANSFER bit set, so iflags=ITEM_IS_NEW (or any other non-transfer iflag word) bypasses the check. Apply the same guard receiver-side. Confirmed: the same PoC (a minimal Python rsyncd that handshakes with CF_INC_RECURSE, sends a no-leading-"." flist, and emits ndx=0 with ITEM_IS_NEW) crashes unpatched 3.4.2 with SEGV_MAPERR si_addr=0x4101a-class in the receiver child; with this guard it exits cleanly with code 2 (RERR_PROTOCOL). The attack surface delta over the sender variant is large: the original was malicious-client -> daemon, this is malicious-server -> any rsync client doing a normal rsync:// or remote-shell pull. Reported by Pratham Gupta (alchemy1729). Co-Authored-By: Claude Opus 4.7 (1M context) --- generator.c | 4 ++++ io.c | 3 +++ receiver.c | 7 ++++++- sender.c | 2 ++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/generator.c b/generator.c index b80eb2e3..38f5ad33 100644 --- a/generator.c +++ b/generator.c @@ -2146,6 +2146,8 @@ void check_for_finished_files(int itemizing, enum logcode code, int check_redo) if (send_failed) ndx = get_hlink_num(); flist = flist_for_ndx(ndx, "check_for_finished_files.1"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = flist->files[ndx - flist->ndx_start]; assert(file->flags & FLAG_HLINKED); if (send_failed) @@ -2174,6 +2176,8 @@ void check_for_finished_files(int itemizing, enum logcode code, int check_redo) flist = cur_flist; cur_flist = flist_for_ndx(ndx, "check_for_finished_files.2"); + if (ndx < cur_flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = cur_flist->files[ndx - cur_flist->ndx_start]; if (solo_file) diff --git a/io.c b/io.c index 8d1cf7f2..2d94c1f4 100644 --- a/io.c +++ b/io.c @@ -1090,6 +1090,9 @@ static void got_flist_entry_status(enum festatus status, int ndx) { struct file_list *flist = flist_for_ndx(ndx, "got_flist_entry_status"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); + if (remove_source_files) { active_filecnt--; active_bytecnt -= F_LENGTH(flist->files[ndx - flist->ndx_start]); diff --git a/receiver.c b/receiver.c index 63e5cedb..0a993e0f 100644 --- a/receiver.c +++ b/receiver.c @@ -467,7 +467,10 @@ static void handle_delayed_updates(char *local_name) static void no_batched_update(int ndx, BOOL is_redo) { struct file_list *flist = flist_for_ndx(ndx, "no_batched_update"); - struct file_struct *file = flist->files[ndx - flist->ndx_start]; + struct file_struct *file; + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); + file = flist->files[ndx - flist->ndx_start]; rprintf(FERROR_XFER, "(No batched update for%s \"%s\")\n", is_redo ? " resend of" : "", f_name(file, NULL)); @@ -604,6 +607,8 @@ int recv_files(int f_in, int f_out, char *local_name) if (ndx - cur_flist->ndx_start >= 0) file = cur_flist->files[ndx - cur_flist->ndx_start]; + else if (cur_flist->parent_ndx < 0) + exit_cleanup(RERR_PROTOCOL); else file = dir_flist->files[cur_flist->parent_ndx]; fname = local_name ? local_name : f_name(file, fbuf); diff --git a/sender.c b/sender.c index 99f431fe..033f87e5 100644 --- a/sender.c +++ b/sender.c @@ -140,6 +140,8 @@ void successful_send(int ndx) return; flist = flist_for_ndx(ndx, "successful_send"); + if (ndx < flist->ndx_start) + exit_cleanup(RERR_PROTOCOL); file = flist->files[ndx - flist->ndx_start]; if (!change_pathname(file, NULL, 0)) return;