mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-09-17 07:38:57 -04:00
Andrew Tridgell
c3ba28c19e
Two shapes a pristine 3.4.4 rrsync transfers, and88cee089broke, both from one cause: the pin opens the argument's CONTENT, when for a sender rsync often only needs to name or describe it. * an in-tree FIFO wedged rrsync before exec. O_RDONLY on a FIFO blocks until a writer appears, so an authorised user naming one could accumulate stuck processes indefinitely. * an in-tree dangling symlink failed the transfer. realpath() resolved it to a missing target and the ENOENT was reported as a detected race, though a dangling link is an ordinary archive entry that rsync transmits by its target string without opening anything. So only a regular file or a directory gets its content opened; anything else keeps the realpath()-validated name. The sender never opens these, it only describes them. The leaf is still spelled beneath a pinned directory. An earlier form of this commit left the bare name for rsync to re-resolve, on the reasoning that 3.4.4 passes it that way -- but that puts every component back in play and reintroduces CVE-2026-53783 for the shape: with an in-tree "dir/target" that is a dangling symlink, flipping "dir" to a symlink pointing outside leaked the outside file's content in 3 of 83 raced pulls. With the parent pinned it is 0 in 104 -- but a race only samples the window, and zero in 104 still leaves a few per cent of per-attempt risk unmeasured, so rrsync-sender-parent-pin closes it deterministically instead: a stub standing in for rsync inherits the pinned descriptor and blocks, the parent is swapped for a symlink out of the tree while it is blocked, and only then does the stub resolve the argument. It reports the in-tree leaf with the pin and the attacker's file without it, so it fails outright if the pin is removed rather than depending on winning anything. A control first proves the swap really does redirect the bare name, or the assertions would prove nothing. Pinning the parent costs nothing here -- pin_dir() opens it O_PATH, so the special file itself is still never opened and a FIFO still cannot block, and whatever the leaf becomes afterwards is reached only from beneath the held one. Which directory that is, sender_pinned_arg() already decides, and for every shape except one it is the immediate parent. The exception is a --relative argument with no client "/./": there the whole argument is the transmitted name, so only the anchor it starts from can be pinned and the components below it stay raceable. That limit predates this commit and NEWS states it; "the parent is pinned" is not true of that one shape. Two boundaries this must NOT cross, each found the hard way: * a trailing "/" or "/." argument keeps its leaf pin: rsync opens that one and does follow a symlink there. Declining it made rrsync-sender-leaf-flip leak the outside directory's content. * the decision is not gated on HAVE_PROC_SELF_FD. It is about what rsync does with the argument, not about whether we can pin it, so gating it left the dangling-symlink failure in place on the BSDs, macOS, Solaris and Cygwin. The shape matrix grows fifo, dangling-symlink and symlink-to-file cases, and now asserts what each delivered entry IS -- kind, symlink target and content -- on every case rather than spot-checking a couple at the end. A name-only comparison is satisfied by an empty directory called "f1", or by the correctly-named but empty symlinks that handing the sender a magic link produced. It still passes against a pristine 3.4.4 rrsync. The FIFO case asserts only that the pull does not hang. What a special file does on the wire is decided by the --no-D that a restricted dir forces on the remote side alone: the sender then omits the old-protocol rdev fields that the client's own -D receiver still reads, so protocol 29 and 30 fail regardless of this change. Verified by running the FIFO case under fakeroot at protocol 29 with and without the parent pin -- it hangs identically either way, so the pinned name is not the cause. That asymmetry is a pre-existing rrsync bug and is tracked separately. (cherry picked from commit1c0bd88f0b)