From ee4f668f295774ddd9764886ddd65014a57906bc Mon Sep 17 00:00:00 2001 From: pterror Date: Fri, 5 Jun 2026 17:24:05 +1000 Subject: [PATCH] receiver: fix NULL deref on the delta discard path receive_data() crashed a receiver that was merely DISCARDING a file's delta stream. discard_receive_data() calls receive_data() with fname == NULL and fd == -1, so size_r == 0 and mapbuf == NULL. A normal block-MATCH token (against a block the basis and source share) then reaches the !mapbuf branch added in 31fbb17d ("receiver: fix absolute --partial-dir delta resume"), which calls full_fname(fname). full_fname() dereferences its argument unconditionally (util1.c: `if (*fn == '/')`), so fname == NULL faults there -> receiver SIGSEGV. This is a normal-operation crash with a stock cooperating sender, not an adversarial one. The generator hands the sender real block sums whenever the basis is readable and we're in delta mode; the receiver only decides to discard afterwards, when its output cannot be produced -- e.g. the destination directory is not writable (mkstemp fails), the basis turns out to be a directory, or a --partial-dir resume is skipped. A MATCH token arriving during that discard hit the NULL deref. The 31fbb17d branch is correct only for a REAL output transfer (fd != -1, fname valid): there, a block match with no mapped basis is a genuine protocol inconsistency (the generator promised a basis the receiver could not open), and honoring it would silently omit those bytes from the verification checksum or leave a hole, so hard-erroring -- and full_fname(fname) -- is right. It conflated that with the discard path. The discriminator is fd, not mapbuf: on the discard path fd == -1 always; on the real-output inconsistency fd != -1. Scope the "no basis file" protocol error to fd != -1 (where fname is non-NULL and full_fname is safe) and, on the discard path (fd == -1), absorb the matched bytes benignly (offset += len; continue) -- symmetric with the literal-token handling just above, and restoring the pre-31fbb17d behavior. The real-transfer inconsistency check is preserved unchanged. --- receiver.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/receiver.c b/receiver.c index cb797841..30855e13 100644 --- a/receiver.c +++ b/receiver.c @@ -423,16 +423,32 @@ static int receive_data(int f_in, char *fname_r, int fd_r, OFF_T size_r, stats.matched_data += len; - /* A block match can only be honored if we actually mapped the - * basis. If we didn't (basis open failed), the sender should - * never have been told a basis existed -- treat it as a protocol - * inconsistency rather than silently omitting these bytes from - * the verification checksum (which yields a spurious failure) or - * leaving a hole in the output. */ + /* A block match with no mapped basis is a protocol inconsistency + * ONLY when we are actually producing output (fd != -1): the + * generator told the sender a basis existed but the receiver could + * not open it, so honoring the match would silently omit these + * bytes from the verification checksum (a spurious failure) or + * leave a hole in the output. Fail cleanly in that case. + * + * On the DISCARD path (fd == -1, fname == NULL) there is no output + * and no verification: discard_receive_data() deliberately drains a + * delta the receiver never intends to write (basis fstat failed, + * basis is a directory, output open failed, batch skip, ...). The + * sender does not know the data is being discarded and streams an + * ordinary delta, so a match token here is NORMAL protocol, not + * malformed. Absorb it benignly (advance the offset and continue), + * as the pre-existing "if (mapbuf)" guards did before this check was + * added in 31fbb17d -- erroring would wrongly break legitimate + * transfers, and full_fname(fname) with fname==NULL would + * dereference NULL (a receiver crash on a normal transfer). */ if (!mapbuf) { - rprintf(FERROR, "got a block match with no basis file for %s [%s]\n", - full_fname(fname), who_am_i()); - exit_cleanup(RERR_PROTOCOL); + if (fd != -1) { + rprintf(FERROR, "got a block match with no basis file for %s [%s]\n", + full_fname(fname), who_am_i()); + exit_cleanup(RERR_PROTOCOL); + } + offset += len; + continue; } if (DEBUG_GTE(DELTASUM, 3)) {