From c3ba28c19edbff1bb45582dbe3a4a0b2c54e0ef7 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 28 Jul 2026 19:28:40 +1000 Subject: [PATCH] rrsync: don't content-open a sender leaf rsync will never open Two shapes a pristine 3.4.4 rrsync transfers, and 88cee089 broke, 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 commit 1c0bd88f0b93f3fe7ab6f9612dab1f420bd422fb) --- support/rrsync | 85 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/support/rrsync b/support/rrsync index 8eba81bc..e23a3061 100755 --- a/support/rrsync +++ b/support/rrsync @@ -320,6 +320,11 @@ def sender_pinned_arg(fd, arg, orig_arg, has_slash, has_slash_dot): # Tie the pinned directory to the inode realpath() validated: resolving # `check` beneath the held fd cannot be redirected above the leaf, so if it # does not reach the same file, something was flipped -- fail closed. + # fd is None for a leaf we deliberately never opened (a symlink, or a + # device/FIFO/socket): there is no inode to compare against, and the leaf + # was never going to be content-opened by the sender either. + if fd is None: + return pinned try: st = os.stat(check, dir_fd=dfd) except OSError as e: @@ -595,12 +600,50 @@ def validated_arg(opt, arg, typ=3, wild=False): # that don't exist yet (receiver-side new dest) os.open # fails -- we skip pinning there; the new-dest race is a # separate concern. - try: + # Only a regular file or directory gets its CONTENT opened. A + # sender needs neither for anything else: rsync transmits a symlink + # by its target string and skips a device/FIFO/socket under the + # forced --no-D. Opening them here is also actively wrong -- + # O_RDONLY on a FIFO blocks until a writer appears, so naming an + # in-tree FIFO wedged rrsync before exec, and a dangling symlink + # resolved to a missing target and was reported as a race. 3.4.4 + # transfers both. These shapes take the parent pin, which is what + # confines them anyway. + # NOT for a trailing "/" or "/." argument: rsync opens that one and + # DOES follow a symlink there, so its leaf pin is load-bearing -- + # rrsync-sender-leaf-flip proves a raced flip leaks the outside + # directory's content without it. + sender_leaf_unopened = False + # Not gated on HAVE_PROC_SELF_FD: this is a decision about what + # rsync does with the argument, not about whether we can pin it, so + # it has to hold on the BSDs, macOS, Solaris and Cygwin too -- where + # otherwise a dangling symlink still resolved to nothing and died. + if (am_sender and opt == 'arg' + and not arg_has_trailing_slash + and not arg_has_trailing_slash_dot): try: - fd = os.open(real_arg, os.O_RDONLY | os.O_NOFOLLOW) + lst = os.lstat(arg) + except OSError: + lst = None + if lst is not None and not (stat.S_ISREG(lst.st_mode) + or stat.S_ISDIR(lst.st_mode)): + sender_leaf_unopened = True + try: + if sender_leaf_unopened: + raise InterruptedError() # jump to the sender-pin branch + try: + # O_NONBLOCK so a special file that raced in after the + # lstat above still cannot block this open. + fd = os.open(real_arg, + os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) except IsADirectoryError: fd = os.open(real_arg, os.O_RDONLY | os.O_NOFOLLOW | os.O_DIRECTORY) + except InterruptedError: + # No CONTENT fd for this leaf -- but it is still named + # beneath a pinned directory below, not re-resolved from + # the tree root. + fd = None except FileNotFoundError: # In --sender mode the path MUST exist (we're reading # from it) -- ENOENT here means the rename-based race @@ -645,7 +688,37 @@ def validated_arg(opt, arg, typ=3, wild=False): # the pin (typically a symlink-flip on the leaf). die('post-realpath open failed (race detected):', orig_arg, e.strerror) - if fd is not None: + if am_sender and opt == 'arg': + logical = arg + if is_absolute_arg: + if logical == args.dir: + logical = '' + elif logical.startswith(args.dir_slash): + logical = logical[args.dir_slash_len:] + if fd is None and sender_leaf_unopened: + # A leaf we deliberately never opened is still spelled beneath + # a pinned directory: leaving the bare name for rsync to + # re-resolve puts every component back in play, which is + # CVE-2026-53783 -- measured at 3 leaks in 83 raced pulls with + # a dangling-symlink leaf whose parent was flipped to point + # outside the tree. It costs nothing here: the directory is + # opened O_PATH, so the special file itself is never opened + # and a FIFO cannot block, and whatever the leaf turns into + # afterwards is reached only from beneath the held one. + # + # WHICH directory is sender_pinned_arg()'s decision, and it is + # the immediate parent for every shape EXCEPT 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 + # --relative limit predates this and is stated in NEWS. + if HAVE_PROC_SELF_FD: + pinned = sender_pinned_arg(None, logical, orig_arg, + arg_has_trailing_slash, + arg_has_trailing_slash_dot) + if pinned != LEAF_PIN_UNUSABLE: + arg = pinned + elif fd is not None: # The inode-pin trick (verify + route the exec'd rsync's open via # the /proc/self/fd magic link) is Linux-only. Where /proc/self/fd # does not exist at all (the BSDs, Solaris, macOS, Cygwin, or a @@ -672,12 +745,6 @@ def validated_arg(opt, arg, typ=3, wild=False): die('post-pin path escaped tree (race?):', orig_arg, pinned_path) if am_sender and opt == 'arg': - logical = arg - if is_absolute_arg: - if logical == args.dir: - logical = '' - elif logical.startswith(args.dir_slash): - logical = logical[args.dir_slash_len:] pinned = sender_pinned_arg(fd, logical, orig_arg, arg_has_trailing_slash, arg_has_trailing_slash_dot)