From: Andrew Tridgell Date: Tue, 5 May 2026 06:48:16 +0000 (+1000) Subject: receiver: add parent_ndx<0 guard, mirroring 797e17f X-Git-Tag: v3.4~11 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=0cf200ecbb8baaf58070d825c4fbd892b4a63b69;p=thirdparty%2Frsync.git 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) --- diff --git a/generator.c b/generator.c index 311e9b78..89f99db4 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 ec34c4b1..c5c14076 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 a487ad5a..3fa68d71 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;