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.
This commit is contained in:
pterror
2026-06-05 17:24:05 +10:00
committed by Andrew Tridgell
parent c14e2258b5
commit ee4f668f29

View File

@@ -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)) {