Resolve every operator/peer-reachable filesystem path one component at a time
through a stack of O_NOFOLLOW-held directory fds, so a symlink swapped in mid-walk
cannot redirect the operation (TOCTOU). Adds the do_*_atfd() wrappers, the held
dirfd cache (held_dfd_for) and do_mkstemp_atfd() for race-safe temp-file creation,
plus secure change_dir()/robust_rename() in util1.c. t_stub.c gains the matching
test stubs.
The run4 0002 fix deferred the in_multiplexed re-arm past the call-out only for
MSG_NOOP and MSG_IO_ERROR (the two fuzzed tags), but the same anti-pattern --
re-arming the re-entry guard before a write-side call-out that can reach
perform_io() -> read_a_msg() -- remained in the sibling cases (Codex review).
Defer it past the call-out in those too:
- MSG_NO_SEND (!am_generator -> send_msg_int): exploitable at a daemon sender,
the same stack-exhaustion recursion as MSG_NOOP.
- MSG_DELETED (!am_generator -> log_delete -> send_msg when am_server): same.
- MSG_DELETED (am_generator -> send_msg): not currently exploitable (the
generator is not am_sender, so send_msg doesn't perform_io), deferred for
uniformity.
MSG_SUCCESS/MSG_REDO are left as-is: successful_send() returns early unless
--remove-source-files and does not reach send_msg()->perform_io(), and the
generator-status path runs only in the (non-am_sender) generator.
(cherry picked from commit f4f1905f5cfd83bd10d4db3b734da34cc43d33b0)
After the '.' separator, read_args delegates argv growth to
glob_expand_module() -> glob_expand()/glob_match(), each of which reserves
glob.argc+1 slots -- room for the entry being added but not for the trailing
NULL that read_args writes after the loop. A daemon client can send a single
post-dot request line whose " mod/" splits land argc on exactly maxargs
(initially MAX_ARGS=1000, then any ENSURE_MEMSPACE doubling boundary), so the
'argv[argc] = NULL' at the end is an 8-byte NULL write one slot past the argv
heap allocation. Reachable from an unauthenticated client; runs after
chroot/setuid in rsync_module so the corruption is in the privilege-dropped
per-connection child.
Ensure the +1 slot before the store. The pre-dot growth check at
'argc == maxargs-1' already leaves the slot but never fires once dot_pos is set.
(cherry picked from commit ec4f7a03c42e93a48fce20d0b7667235564f9248)
read_a_msg() sets iobuf.in_multiplexed = -1 on entry so that any perform_io()
reached while the message body is being consumed will not re-enter read_a_msg()
(IN_MULTIPLEXED_AND_READY tests > 0). The MSG_NOOP and MSG_IO_ERROR handlers
reset it to 1 *before* calling maybe_send_keepalive() / send_msg_int(), both of
which can hit perform_io(PIO_NEED_MSGROOM). With more frames already buffered
(iobuf.in.len > 512), perform_io() then loops back into read_a_msg() -- each
frame stacks a fresh BIGPATHBUFLEN local, so a hostile peer that floods MSG_NOOP
at a daemon sender (or MSG_IO_ERROR at a daemon receiver child) exhausts the
stack.
Move the reset after the call-out so the guard stays armed across the write-side
flush. The outer caller continues draining input once we return.
(cherry picked from commit 5d2794fca309dd558f68b21bff6d3e9728e58237)
Phase 1 of add_implied_include() decides whether a '\' is escaping a wildcard
via !strchr("*[?", cp[1]). strchr() returns a pointer to the string literal's
NUL terminator when cp[1]=='\0', so a trailing backslash matches neither branch:
it is emitted into new_pat via the fall-through but backslash_cnt is not
incremented. The phase-2 recurse/xfer_dirs '/**' rule then allocates
arg_len + backslash_cnt + 3 + 1 and doubles every '\' in new_pat, including the
uncounted trailing one, so the closing '*p = '\0'' lands one byte past the heap
buffer.
Reachable on a standard 'rsync --daemon' deployment (am_server==0 at the
per-module parse_arguments(), so trust_sender_args stays 0) by a remote
unauthenticated client that sends '-r --files-from=-' and a files-from name with
both an interior and a trailing backslash (e.g. 'a\b\'). Count the trailing
backslash so the allocation reserves the doubled slot.
(cherry picked from commit e377e7dc4f3e752723e5fbf9623a8315034e49ab)
In daemon-as-sender mode, send_files() leaves mbuf NULL for an empty local file
(st.st_size == 0). receive_sums() then derives s->flength from the peer's
sum_head (count * blength), so a hostile client that sends a non-empty sum_head
against that empty file drives the append_mode==2 verify loop into
map_ptr(NULL, ...), dereferencing the NULL map. The diminished-file guard
compares against F_LENGTH(file) (the daemon's own flist size, also 0), not the
peer-supplied flength, so it does not catch this.
Clamp s->flength to len before the verify loop -- we cannot checksum bytes we do
not have, and last_match must not run past len for the trailing matched() flush.
(cherry picked from commit 43f43c92d3414f340029ddf7ddc316dc4d4e3e7b)
The check_want_i adjacent-match optimisation substitutes i = want_i after
verifying sum1 and sum2, but -- unlike the main chain loop and the aligned_i
path -- did not re-check s->sums[want_i].len == l. A hostile peer acting as the
generator against a daemon-as-sender can set s2length=0 (read_sum_head only
enforces >= 0, so the sum2 memcmp is a no-op) and craft count=2, blength=len+1,
remainder=len with both sum1 set to the file's rolling checksum. The chain head
(.len=remainder=len) passes the l==.len gate; want_i=0 (.len=blength=len+1) is
then substituted unchecked. After matched(), offset advances to len, k =
MIN(blength, len-offset) = 0, map_ptr(...,0) returns NULL, and the rolling-
checksum trim dereferences map[0] -- a remote unauthenticated SEGV of the
per-connection daemon child against any module serving a non-empty regular file.
Add the missing length re-check, matching the chain loop and aligned_i path.
(cherry picked from commit aad82d77aae098c8d8529f69614a0ffcbfbbb5d3)
C99 snprintf returns the would-have-written length on truncation, so when a
daemon client sends an oversized --max-size/--min-size/--bwlimit/--block-size/
--max-alloc value (each arg can be up to BIGPATHBUFLEN-1 = 5119 bytes via
clientserver.c:read_args), len at the failure: label can far exceed
sizeof err_buf (200). The trailing
err_buf[len] = '\n';
err_buf[len+1] = '\0';
then write 0x0A,0x00 at a byte-precise attacker-chosen offset up to ~4.9KB
past err_buf into adjacent .bss. The same happens via the second snprintf
when the first lands len in [~140,189] and the (min/max: ...) suffix is
truncated.
Clamp len to sizeof err_buf - 2 before the trailing writes, covering both
paths. Reachable from an unauthenticated client against any daemon module
before chroot/privilege-drop; introduced in 66ca4fc97b (2020-07-10).
(cherry picked from commit 3b81103d7ec98145710f8845dd676ba34458169a)
A daemon receiver (am_server=1, am_generator=0, send_msgs_to_gen=1) that
receives MSG_INFO/MSG_ERROR/MSG_ERROR_XFER/MSG_WARNING from the wire calls
rwrite() with is_utf8=!am_generator=1 (io.c:read_a_msg). The
send_msgs_to_gen branch in rwrite() then asserted !is_utf8, giving a
remotely-reachable abort of the per-connection receiver child.
Legitimate senders never put these tags on the wire (they write to their
own terminal), so this never fired on real traffic. Under NDEBUG the
assert compiles out and the bytes are forwarded raw to the generator,
which logs them with is_utf8=0 through filtered_fwrite -- safe. Match
that behavior in debug builds: drop the assert and document why is_utf8
may legitimately be set here.
(cherry picked from commit e5a89f3372716ee42fcb145d7a65f9037391e074)
A '. file' (merge) directive inside a filter file makes
parse_filter_str() call parse_filter_file() on the named file, which
in turn feeds each line back through parse_filter_str(). Nothing
bounds the depth, so a file that merges itself recurses until the
stack guard page is hit.
A malicious client of a writable daemon module can trigger this in a
single connection: send '--filter=": .merge"' with --delete-after,
upload a '.merge' file whose only line is '. .merge', and the
generator's per-dir merge handling in push_local_filters() then opens
and re-parses that file forever, crashing the forked daemon child.
Cap the nesting at MAX_MERGE_DEPTH (32). At the cap, mirror the
existing failed-open handling: abort with RERR_FILEIO under a fatal
(operator-supplied) merge, otherwise log an FERROR and drop the
offending merge so the transfer continues rather than turning a
planted filter file into a guaranteed abort.
(cherry picked from commit c6c585afdf0e2ae88ae57fd46ff9e4a7f4f442fc)
In incremental-recursion mode the receiver frees each flist as it is
finished with. When the last one goes, flist_free() sets first_flist
to NULL, flist_cnt to 0 and pool_destroy()s the shared file pool, but
the global dir_flist is left untouched: its files[] array still points
into the now-destroyed pool.
A malicious sender can then send an NDX_FLIST_OFFSET-encoded directory
index. read_ndx_and_attrs() bounds-checks dir_ndx against the *stale*
dir_flist->used and calls recv_file_list(), which:
- reads/writes the freed dir_flist->files[dir_ndx] entry
(FLAG_GOT_DIR_FLIST), and
- because first_flist is NULL, gets ndx_start == 1 from flist_new(),
so re-creates dir_flist as a fresh empty FLIST_TEMP list, and then
- dereferences dir_flist->files[dir_ndx] (uninitialised malloc heap)
in the dirname check, crashing in f_name().
On a stock glibc the 256 KiB realloc is mmap-zeroed, so the uninit
deref is caught by the existing !d guard and turns into a clean
RERR_UNSUPPORTED exit, but the use-after-free write into the destroyed
pool happens regardless.
A sub-flist marker after every flist has been freed is never valid
protocol, so refuse it up front before touching the stale dir_flist.
(cherry picked from commit 382d7df9a23b976ea6f09598ecea66f269aa213d)
The F_HL_GNUM/F_HL_PREV optional-extras slots ("non-dirs") and the
F_DIR_DEV_P slots ("receiver dirs") occupy the same OPT_EXTRA offsets;
the macros assume an entry is one or the other. recv_file_entry()
suppressed FLAG_HLINKED for symlinks and specials/devices on platforms
that cannot hard-link those, but never for directories, so a malicious
sender could mark a directory entry XMIT_HLINKED.
With -H -x and inc_recurse the generator then writes
major/minor(st_dev) into F_DIR_DEV_P(file) (generator.c:1679),
clobbering what the hard-link code reads as F_HL_PREV/F_HL_GNUM. A
forwarded MSG_NO_SEND for that ndx pushes it onto hlink_list
(got_flist_entry_status() only checks F_IS_HLINKED), and
check_prior() reads F_HL_PREV(file) == major(dev) — typically 0.
flist_for_ndx(0) returns first_flist (ndx_start-1 is the parent
sentinel), and flist->files[0 - ndx_start] underflows the files[]
array by one slot, then dereferences the resulting wild pointer.
Directories cannot be hard-linked on POSIX; do not honour XMIT_HLINKED
on an S_ISDIR entry.
(cherry picked from commit 63be23ed575a24430b058d54724a643c89344b5b)
log_delete() builds a placeholder file_struct with only
(file_extra_cnt + 2) extras — enough for the REQ_EXTRA fields and the
first two OPT_EXTRA slots, but not for the SUM_EXTRA_CNT slots that
F_SUM() reaches into. When the active out-format / log-format contains
%C and always_checksum is on, log_formatted() called sum_as_hex() on
F_SUM(file) for the placeholder, reading flist_csum_len bytes that begin
before the new_array0() allocation and hex-encoding them into the log
line / FCLIENT message.
A daemon receiver can be driven onto this path entirely from the wire:
the client args line can include --out-format='%o%C' and -c, and
read_a_msg() accepts a multiplexed MSG_DELETED from the peer; a payload
of MAXPATHLEN or more bytes skips the send_msg() forward and lands in
the FCLIENT log_formatted() call with the attacker-supplied format.
A deleted entry has no checksum to print, so fall through to the
existing blank-fill branch when ITEM_DELETED is set. This also covers
the legitimate generator-side log_delete() callers when the daemon's
configured "log format" contains %C.
(cherry picked from commit 84728d81ef92268fb549c24f393c18a8e8eadfd9)
A malicious sender can set XMIT_HLINKED on a file-list entry even when
the receiver was not invoked with -H. recv_file_entry() only reserves
the (inc_recurse+1) hard-link extras slots when preserve_hard_links is
set (the gate at create_object), but it set FLAG_HLINKED on the struct
unconditionally from the peer's xflags. HLINK_BUMP() then offsets
F_SUM() and F_RDEV_P() past the start of the pool_alloc()'d region,
and the subsequent read_buf(f, F_SUM(file), flist_csum_len) writes 16
attacker-controlled checksum bytes 8 bytes below the allocation — into
the adjacent file_struct in the same pool extent.
Vanilla ASAN does not see this because lib/pool_alloc.c bumps inside a
single 32 KiB malloc; it surfaces as use-after-poison once the pool is
instrumented with ASAN_POISON_MEMORY_REGION/ASAN_UNPOISON_MEMORY_REGION.
Mirror the gating already present at the extra_len computation and at
the F_HL_GNUM/F_HL_PREV write site: only set FLAG_HLINKED when
preserve_hard_links is enabled. This also closes the symmetric
F_RDEV_P() underflow for device entries and the F_SUM(first)/
F_RDEV_P(first) reads in the first_hlink_ndx copy path.
(cherry picked from commit adfae919876c9f3454ce12a1b7becbed47415ff7)
A peer can send duplicate file-list entries; flist_sort_and_clean() then
clear_file()s one of them (basename and mode zeroed), but its slot stays
indexable. If the transfer phase later sends an ndx for that slot,
recv_files()/send_files() pick the cleared file_struct and f_name() on it
returns NULL. In recv_files() that NULL flowed straight into the daemon
filter check and set_file_attrs() → full_fname(), which dereferences its
argument: a remote NULL-pointer crash on any anonymous writable module
with -X negotiated.
Reject an inactive entry at the point both receiver and sender resolve
ndx → file_struct, and harden full_fname() against a NULL argument as a
backstop for the many error-reporting call sites that feed it an f_name()
result.
Found by fuzzing the daemon protocol over stdio.
(cherry picked from commit 8c70bff2daf5ba06a3b42ef58b9860b564c98ce9)
Under inc_recurse the receiver appends every received directory entry to
dir_flist before flist_sort_and_clean() runs. If the peer sends entries
that the dedup/prune pass then clear_file()s, the cleared file_struct
(basename and mode zeroed) remains in dir_flist->files[]. A later
sub-flist tagged with that dir_ndx passed the existing >= used bounds
check, and the dirname-validation strcmp() in recv_file_list() then
called f_name() on the cleared entry — which returns NULL — and
dereferenced it: a remote NULL-pointer crash on any anonymous writable
module.
Reject a dir_ndx whose entry is no longer active alongside the existing
range and duplicate-flist checks, and guard the strcmp() against a NULL
f_name() result as defence in depth.
Found by fuzzing the daemon protocol over stdio.
(cherry picked from commit a900b25008151a8a8ec1c34aa7e2914b23416019)
With inc_recurse, a non-first hard-link entry carries first_hlink_ndx as
its gnum. recv_file_entry() bounds it as [0, ndx_start + used) but does
not require that a value below ndx_start was ever declared via
XMIT_HLINK_FIRST in an earlier flist. When such a gnum reaches
match_gnums() it is absent from prior_hlinks, and the new-node branch hit
assert(gnum >= hlink_flist->ndx_start) — a peer-controlled abort() on
default builds.
Replace the assert with a clean RERR_PROTOCOL exit. In NDEBUG builds the
old code silently tolerated the bogus gnum (it is only used as a hash
key, not an index), so there is no memory-safety exposure either way; the
change just turns SIGABRT into a normal protocol error.
Found by fuzzing the daemon protocol over stdio.
(cherry picked from commit 56b40f0fae4c85f46bfba2aef872a7ce10565fbc)
The bump allocator hands out chunks from one big malloc, so AddressSanitizer
cannot see a write that underflows one chunk into its neighbour -- e.g. a
miscomputed F_SUM() that reaches before a file_struct's extras (the
FLAG_HLINKED-without-H bug). Under ASan, poison a small redzone just below
each returned chunk (allocations grow downward from the top of an extent) so
such an underflow becomes a hard use-after-poison report; unpoison a whole
extent whenever its space is reused (reset/reclaim) so legitimate later
allocations never trip over an old redzone. Compiled out entirely without
-fsanitize=address.
Validated: a deliberate write 12 bytes below F_SUM is caught as a
use-after-poison; the existing suite (hardlinks/inc-recurse, daemon, xattrs,
merge) is clean under ASan, so the redzone introduces no false positives.
Also add a require_asan() test helper (mirrors require_tcp) for tests that are
only meaningful under an ASan build.
(cherry picked from commit 402962b7cdea8afb27c0e6e198fef62172c751e3)
main()'s line parser stepped through the fgets() buffer with `*++s` in
three places without first checking for the terminating NUL, so a test
line whose last token runs to the end of the buffer (e.g. a final line
with no trailing newline) could advance s past the NUL and read out of
bounds.
Guard the flag-separator check and rewrite the two whitespace-skip loops
so they never step past the NUL. No behaviour change for well-formed
input: the existing wildtest.txt still passes.
Fixes#776
Reported-by: vikk777 (@vikk777)
rwrite()'s daemon/logfile branch did strlcpy(msg, buf, MIN(sizeof msg, len+1)),
but strlcpy() scans the whole source with strlen(); buf is the data buffer from
read_a_msg() (io.c) holding exactly len bytes of a forwarded MSG_* payload with
no NUL terminator, so strlen() reads past the message into uninitialised stack.
Copy exactly len (bounded) bytes with memcpy() and NUL-terminate, matching the
(buf, len) contract the rest of rwrite() already honours. Behaviour is
unchanged for the NUL-terminated callers; the over-read is gone.
this fixes a valgrind error where we could read an uninitialised sx.st
field when we don't fill the stat data.
Thanks to report from Michael Mess <michael@michaelmess.de>
set_file_attrs() resolved the entry's directory into a held dirfd (for the
chmod/chown/times *at ops) but its initial link_stat() -- and the post-chown
setuid/setgid re-stat -- still went through x_lstat()/do_lstat_at(), re-walking
the full path per file. Do the stat with link_stat_at() against the held dirfd
(mirroring gen_entry_stat, guarded on am_root >= 0 for the fake-super xattr
fold) so the directory is opened once and reused.
When configure detects a patched libacl exporting acl_get_file_at /
acl_set_file_at / acl_delete_def_file_at (new HAVE_LIBACL_AT; ACL_1.3), route the
receiver's race-safe ACL get/set/delete through them instead of lib/acl.c's
xacl_* -- held fd via AT_EMPTY_PATH, dirfd+leaf via AT_SYMLINK_NOFOLLOW, reusing
the existing unpack_smb_acl/pack_smb_acl conversion. libacl's /proc/self/fd
compat makes this race-safe on every Linux kernel.
lib/acl.c stays as the fallback for a stock libacl (macro undefined -> zero change
there); it already closes the same pre-6.13 parent-symlink-race gap via its own
/proc/self/fd compat, so the two paths are equivalent in safety and prefer libacl
only when its newer bindings are present. -VV reports ACL-libacl-at and keeps
"ACL_at": true.
A stock -lacl lacks these symbols, so HAVE_LIBACL_AT is undefined on real distros
and the build is byte-identical to the lib/acl.c path until libacl ships ACL_1.3;
build against the patched lib via CPPFLAGS/LDFLAGS pointing at its install prefix.
On a kernel without the Linux 6.13 *xattrat syscalls, xacl_*_at() returned ENOSYS
so acls.c fell back to the unconfined path-based sys_acl_set_file() -- the
long-standing parent-symlink race for --acls on every pre-6.13 Linux (the
receiver applies a received ACL to a leaf an attacker raced to a symlink ->
arbitrary-ACL LPE; acl-symlink-race only documented it as a residual there).
Add a /proc/self/fd compat to the dirfd dispatchers: when the *xattrat syscall is
absent (runtime ENOSYS or unbuilt), address the leaf as
/proc/self/fd/<dirfd>/<leaf> and use the l*xattr (no-follow-leaf) calls. The
/proc/self/fd/<dirfd> magic symlink resolves to the pinned parent inode (a raced
parent symlink can't redirect it) and l*xattr does not follow a raced leaf
symlink, so the dirfd+leaf ACL ops are race-safe on every Linux kernel with
procfs -- no libacl dependency and no 6.13 requirement. xacl_at_available() now
returns true when the syscalls OR the /proc compat are usable, so acls.c takes
the race-safe dirfd path instead of the path-based fallback (now only the BSDs /
a /proc-less namespace). The held-fd path (fsetxattr) is unchanged.
Verified on ubuntu-2004 (kernel 5.4, HAVE_XATTRAT_SYSCALLS=0): "ACL_at": true and
acl-symlink-race PASSES while asserting (a vacuous pass before); no regression on
a 6.13+ host (native syscalls unchanged) or t_acl.
When the receiver cannot pin an entry with a held fd and the kernel has no
*xattrat syscalls (pre-6.13 Linux, the BSDs, Solaris, ...), the hardened ACL
path previously skipped the apply with a warning and reported success -- it
silently left the destination ACL stale, so a changed or revoked source ACL
did not propagate yet rsync exited 0.
Per project policy, prefer the documented --acls functionality over refusing it
on platforms that cannot offer the race-safe primitive (Linux 6.13+ takes the
secure xacl_*_at() path, and an operator who needs the guarantee can move
there). The get / default-ACL-delete / set fallbacks now fall through to the
path-based sys_acl_*file() calls -- the long-standing 3.4.x behaviour -- which
restores correct ACL application at the cost of the parent-symlink race that is
unavoidable on those platforms.
Tests: a new acls-unpinnable test asserts a received ACL updates (and removes a
stale grant from) a no-owner-read (0300) destination directory -- validated on
the path-based fallback via a forced-no-xattrat build and on the xattrat path;
its non-Linux / proto-29 skips are added to the per-platform expect-skip lists.
acl-symlink-race PASSES with a note (rather than skipping) where -VV reports no
race-safe primitive, so its skip set stays kernel-independent -- the same
workflow's RSYNC_EXPECT_SKIPPED is shared by no-xattrat and 6.13+ boxes.
The secure_relpath hardening converted set_xattr()/rsync_xal_set() to
operate on a held, O_NOFOLLOW-opened fd so a parent-symlink race can't
redirect the metadata apply, but copy_xattrs() was missed: it still wrote
each attribute via sys_lsetxattr(dest, ...) on a path. lsetxattr does
not follow the leaf symlink, but the kernel's path walk follows symlinks
in parent components.
All three callers pass peer-influenced destination paths, and two of them
called copy_xattrs() right after closing the fd the secure walker had
opened on that same path:
util1.c copy_file() -- close(ofd) then copy_xattrs(source, dest)
generator.c back_file -- close(f_copy) then copy_xattrs(fname, backupptr)
generator.c alt-dest dir -- copy_xattrs(fnamecmpbuf, fname)
So on a non-chrooted daemon with -X plus --backup/--copy-dest/EXDEV-copy,
an attacker who races a parent-directory swap into the destination or
backup path between the close and the setxattr could set xattrs on a file
outside the module root (security.capabilities if root).
Give copy_xattrs() a dest_fd argument and use sys_fsetxattr() when one is
held, mirroring the fd>=0 ? fsetxattr : lsetxattr pattern already used by
rsync_xal_set/set_xattr. Reorder the two regular-file callers to set the
xattrs before dropping ofd/f_copy, and add a gen_entry_copy_xattrs()
helper that pins the directory case via held_dfd_for + openat(O_NOFOLLOW)
the same way set_file_attrs() already does.
(cherry picked from commit 2f8142f39d1acef8fa08f394f86d669b6673dc82)
set_file_attrs() holds an O_RDONLY|O_NOFOLLOW fd (held_fd) for the entry's
regular-file/dir/FIFO inode so metadata is applied to the pinned inode rather
than by re-resolving the path, which a parent-symlink race could redirect.
- xattrs: the set/remove/list of -X xattrs use the f-variant calls on held_fd
(lib/sysxattrs gains the wrappers); a raced FIFO can't block the open
(O_NONBLOCK).
- ACLs (real root): get_acl_fdat()/set_acl_fdat() apply the ACL via lib/acl.c's
fd path (xacl_*_fd), or setxattrat(AT_SYMLINK_NOFOLLOW) on the dirfd+leaf for
a socket/device, covering both the access and default ACL. pack_smb_acl()+
change_sacl_perms() still build the entries, so the bytes written match
acl_set_file() exactly. With no safe primitive we skip rather than re-resolve
the path. The legacy path-based libacl calls remain only for the cross-tree
(no held dirfd) and non-Linux cases.
- --fake-super ACL-as-xattr writes/deletes also go through held_fd, closing the
last path-based metadata write under fake-super.
POSIX ACLs are stored by the kernel as the system.posix_acl_{access,default}
xattrs. lib/acl.c serializes that wire format and operates on it via
fgetxattr/fsetxattr on a held O_NOFOLLOW fd -- or getxattrat/setxattrat
(AT_SYMLINK_NOFOLLOW) on a dirfd+leaf -- giving a symlink-race-safe ACL
primitive that, unlike libacl's access-only acl_get_fd/acl_set_fd, also covers
the default ACL.
It is self-contained (no libacl, no rsync globals): it speaks a neutral
(tag, perm, id) entry array, so t_acl can compare it directly against the system
libacl as an oracle. configure gains SUPPORT_ACL_FD (POSIX ACLs + the xattr
header, independent of the -X feature) and an optional HAVE_XATTRAT_SYSCALLS
probe (the *xattrat syscalls, Linux 6.13+); the fd path needs neither.
t_acl exercises every op rsync needs in both directions (set via lib -> read via
libacl and vice versa), round-trips, the default-ACL delete, errno
discrimination, and the NOFOLLOW leaf refusal. It self-skips (77) without
SUPPORT_ACL_FD or on a filesystem lacking ACL support.
Backport of the upstream #1002 fix. The simple_recv_token() literal-run
cap added with the GHSA-g37v-g3gj-pmwq hardening rejects peers that use a
64k block size (e.g. the acrosync library / iOS PhotoBackup) with
"invalid uncompressed token length 65536". The residue loop already
reads the run CHUNK_SIZE bytes at a time, so read_buf never overruns the
static buffer; drop the cap to restore interop. The compressed-token
MAX_TOKEN_INDEX overflow cap is unchanged.
copy_altdest_file() cleaned up after a failed --copy-dest/--link-dest
copy with a bare unlink(copy_to). copy_to is the in-tree destination
name (or a tmp name in the same directory), whose parent components are
peer-named and can be raced to a symlink between copy_file() opening the
path and this cleanup. Every other generator-side unlink already goes
through do_unlink_at() so the secure-relpath walker resolves each
component with O_NOFOLLOW; do the same here.
(cherry picked from commit 518f7e1575cfcf6f8deaac6ea8ae1e48134c463d)
The receiver and change_dir confinement for a /./ inner-module chroot was
backported, but the generator still stat'd a --compare/copy/link-dest basis
with plain link_stat(basis_dir/fname), following a symlinked parent that
points outside the inner module (the kernel chroot confines only the outer
path) -- so an outside file's metadata could suppress (or forge a basis
for) the transfer. Route the alt-dest stat through basis_link_stat(),
which resolves the parent via secure_relative_open() and stats the leaf
with link_stat_at() for the inner-module-chroot daemon; plain link_stat()
everywhere else (the non-chroot daemon sanitizes basis paths, and a local
receiver must still follow an operator's --link-dest=../backup).
Closes chroot-alt-dest-inner-module (the openbsd per-component-resolver
case that the secure_relative_open/sanitize path masked elsewhere).
The earlier copy-unsafe-links denial left the rrsync wrapper short of the
3.5.0 restricted-dir hardening, so a daemon-side rrsync still followed a
symlinked --log-file, created device/special files, and had a
realpath-vs-exec TOCTOU. Replace support/rrsync with the fully-hardened
3.5.0 wrapper (version-agnostic; verified functional with this rsync):
- safe_open_logfile(): open the log file O_NOFOLLOW + S_ISREG + inode
re-check so a planted symlink/special is refused;
- force --no-D in a restricted (non-"/") dir so `rsync -a` strips device/
special semantics instead of creating them;
- inode-pin each realpath-validated arg via O_PATH + /proc/self/fd/N
(where available) so the exec'd rsync can't be race-flipped after the
check; fall through unpinned where /proc/self/fd is unavailable.
Tests: rrsync-logfile-symlink, rrsync-specials-denied, rrsync-symlink,
rrsync-copy-unsafe-links-denied, rrsync-archive-mode.
Deny --copy-unsafe-links inside a restricted (non-"/") rrsync directory (it
dereferences a symlink pointing outside the tree and copies the referent
out -- exfiltration), and don't abort the run when flock() is unavailable.
Test: rrsync-copy-unsafe-links-denied.
Validate the hostname passed to the rsync-ssl helper (reject option-injection
/ shell-metacharacter hostnames) and require a CA certificate in stunnel mode
so the server certificate is actually verified.
Tests: rsync-ssl-hostname-validation, rsync-ssl-stunnel-ca-required.
The daemon picks the challenge-response auth digest from the client's
negotiation, or -- for a peer that sends no digest list (any rsync before
3.2.0, including the openrsync that ships with macOS) -- falls back to md5
(protocol >= 30) or md4. The negotiation is unauthenticated, so an on-path
attacker can also rewrite it to force md5/md4 between two modern peers. A
captured (challenge, response) pair is then far cheaper to brute-force
offline against a weak digest, recovering a guessable shared secret.
Add a per-module (or global) "auth digest = NAME" parameter. After the
digest is negotiated, auth_server() refuses the connection (before issuing
the challenge) when the negotiated digest is weaker than NAME, comparing by
position in the strongest-first valid_auth_checksums list. Because the
check is on the resolved digest, it also catches the md5/md4 fallback, so an
old or list-omitting client cannot slip under the floor. A floor naming a
digest this build lacks is fail-closed. Default off, preserving
compatibility with older clients.
Documented in rsyncd.conf(5) and SECURITY.md. The test covers the strong-
client accept, the unknown-floor fail-closed, and -- with a bundled old
static rsync -- the real md5-downgrade refusal (plus a no-floor control).
(cherry picked from commit 1ba7951c; backport drops the SECURITY.md policy
section, which is 3.5.0-specific, and the _test.py, which the stable harness
does not run.)
- io: bound daemon argument lists so a malicious daemon client cannot grow
argv without limit (DoS);
- socket: bound the PROXY CONNECT request and proxy response header lines;
- daemon: require a trusted-proxy host list for "proxy protocol = true"
(reject untrusted proxy peers, fail-closed), and warn at startup when the
trusted-proxy list is unset so the fail-closed behaviour is not silent.
Tests: daemon-argv-limit, proxy-connect-request-too-long,
proxy-response-header-too-long, proxy-protocol-trusted-peer.
- log: open the --log-file / daemon log file through
safe_open_no_attacker_symlinks() so a planted symlink can't redirect the
privileged log write;
- batch: open the --write-batch/--read-batch files the same way (+ an
S_ISREG check), and restore O_BINARY / an O_CLOEXEC fallback on the batch
opens.
Tests: log-file-symlink, batch-file-symlink.
A non-daemon receiver named its own destination and reached it with a
plain chdir(), so a parent component an attacker raced from a real
directory to a symlink was followed -- a root-run transfer's CWD escaped
to an attacker-chosen tree before any write. Resolve the operator-named
dest (absolute and relative) through safe_open_no_attacker_symlinks() +
fchdir: still follow the operator's/root's own symlinked dest (the
/backup -> /mnt/disk admin pattern) but refuse one owned by another uid.
Tests: symlink-race-dest, symlink-race-relative-dest, dest-symlinked-dir.
A daemon module with a /./ inner boundary (use chroot=yes, path=.../.inner)
relies on the inner module root, not the kernel chroot, as its trust
boundary: the chroot confines the outer path, but a symlink inside the
module can point to a sibling that is inside the chroot yet outside the
inner module. Extend the receiver finish/rename gate (use_secure_symlinks),
change_dir()'s confined fchdir, and the basis-open gate from
"am_daemon && !am_chrooted" to "am_daemon && (!am_chrooted || module_dirlen)"
so an inner module gets the same confinement as a non-chroot module.
secure_relpath_active() already carries the inner-module clause.
Tests: chroot-basis-forge/receiver-write/special-inner-module.
parse_filter_file() opened operator- and (via per-directory merge files
like .cvsignore) sender-controlled paths via plain fopen() with no symlink
defense: an unprivileged source-tree owner could plant a dir-merge file as
a symlink to a root-readable file, leaking its content through the filter
parser, or redirect an --exclude-from/--files-from open via a planted
parent. Open via safe_open_no_attacker_symlinks(O_RDONLY)+fdopen(), which
refuses symlinks anywhere in the path unless owned by uid 0 or our euid.
Tests: excludefrom-symlink, filter-merge-symlink.
open_anchor_dirfd() now dups the identity-pinned module_dirfd when the
anchor is the served module root, instead of re-resolving the absolute
module_dir with openat(AT_FDCWD, ...) -- which re-traverses the module's
ancestors as the dropped-privilege module uid and EACCESes when the module
sits under a non-traversable parent (a 0700 home). Functionally identical
(same inode), but privilege-drop-safe. syscall.o links into the t_* test
helpers without clientserver.o, so add a module_dirfd stub to t_stub.c.
Test: daemon-module-private-parent.
Route the daemon's operator-supplied path opens through
safe_open_no_attacker_symlinks() / a pinned parent dir so a local attacker
cannot plant a symlink at one and redirect the daemon's privileged
read/write: motd, --early-input, lock and --config paths
(clientserver/connection/params); the --password-file and daemon secrets
file, fstat'ing the opened fd (authenticate); and the pid-file parent dir.
Tests: daemon-config-symlink, early-input-symlink, daemon-secrets-file-symlink,
password-file-symlink, daemon-module-chdir-symlink.
- safe_open_no_attacker_symlinks(): open an operator-supplied path
refusing to traverse any symlink (parent or leaf) not owned by uid 0 or
our euid -- a trusted-owned symlink is still followed, an untrusted one
fails ELOOP. Used by the daemon's operator-path opens.
- confine the daemon module-root chdir under use-chroot=no so a planted
symlink at the module path is not followed.
The directory-scan confinement resolves the opendir and the per-entry stat
through the held scan dirfd, but readlink_stat() still read the symlink target
with a path-based do_readlink(): a raced parent symlink could redirect that read
and leak an out-of-tree symlink target for a same-name entry.
Add do_readlink_atfd() (readlinkat via the held dirfd) and scan_readlink(), which
routes the read through scan_dirfd for an entry directly in the scan dir (the
same gate scan_link_stat uses), else falls back to do_readlink(). Completes the
enumeration confinement's symlink-target leg.
The sender/daemon enumerated each source directory with a plain opendir()
on the full accumulated path -- not through the secure resolver -- so the
enumeration itself was unconfined: a parent component raced into a symlink
after the scan let the listing (and per-entry lstat) escape the transfer
root / module.
- secure_opendir() resolves the directory through the secure resolver and
turns the held fd into a DIR* via fdopendir(); the daemon anchors it at
the identity-pinned module_dirfd (captured at module setup while the
daemon was positioned at the module root, before any privilege drop);
- the per-entry scan stats through the opendir fd (scan_dirfd) with
fstatat(), so the listing can't be redirected out of the tree.
Closes daemon-subdir-climb-symlink and sender-scan-dir-escape.
A local/non-daemon sender opened each file's content by path
(do_open_checklinks -> do_open_nofollow), O_NOFOLLOWing only the leaf, so
a parent component raced from a real directory to a symlink after the
file-list scan was followed -- a higher-authority sender (root nightly
backup, or a victim copying an attacker's tree) read and copied a file
from OUTSIDE the source tree.
For default symlink handling (no -L/--copy-unsafe-links/-k follow), open
the content through secure_relative_open(NULL, fname, O_RDONLY|O_NOFOLLOW)
anchored at the transfer root change_pathname() chdir'd into, so a parent
that escaped the tree is refused. The symlink-following modes keep
following. The daemon sender was already module_dir-anchored.
Closes symlink-race-source.
The secure-rename and secure-symlink/mknod regression tests drive these
unit helpers (which exercise do_rename_at()/do_symlink_at()/do_mknod_at()
directly), mirroring the existing t_secure_relpath/t_chmod_secure helpers.
Built by `make check-progs`.
Enables: rename-mixed-parent-symlink-race, rename-mixed-parent-escape-poc,
symlink-mknod-fakesuper-symlink-race.
set_file_attrs() and the generator chmod a received entry by name relative to
the held parent dirfd. A plain fchmodat(dfd, name, mode, 0) follows a
final-component symlink an attacker could swap in, escaping the confined tree.
do_fchmodat_nofollow() now stats the leaf NOFOLLOW and, for a regular file,
directory or FIFO, pins it with openat(O_RDONLY|O_NOFOLLOW) and fchmod()s the
held fd: leaf-safe, works on every kernel, and -- unlike the bare fchmodat2()
syscall -- goes through fchmod(), which LD_PRELOAD interceptors such as fakeroot
wrap. A symlink leaf is refused (ELOOP). Sockets/devices and any open failure
fall to fchmodat(AT_SYMLINK_NOFOLLOW), then the raw fchmodat2() syscall as a last
resort; if none is available we skip with a warning rather than fall back to a
leaf-following chmod. Routes do_chmod_at() and do_chmod_atfd() through it.
(Backport of master 463aaa77; the openat2_usable() runtime probe and the
copy_file confinement-gate widening from that commit are omitted -- the former
needs infrastructure not on this branch, the latter is already present.)
The held-directory cache kept only the single current leaf directory, so
moving to a sibling re-resolved the whole path from the anchor and re-opened
every ancestor. The generator and receiver don't recurse -- they iterate a
path-sorted file list (iteration == DFS) -- so there was no call stack holding
the ancestors.
Keep the whole current ancestor chain open as pinned dirfds, keyed on path
components, and on each resolution reuse the longest common prefix, popping
only the divergent tail. Each directory is then opened once while we are
inside its subtree. get_dir_fd() (receiver/generator) and held_dir_path_fd()
(sender) share the stack; ds_descend() still resolves each component, so
in-tree dir-symlink following and confinement are unchanged.
change_dir() drops the cwd-relative stack on any real chdir -- the one
invalidation needed now that the cache is content-keyed, not pointer-keyed.
The per-chunk reset_dir_fd_cache() calls in receiver.c/generator.c are gone:
they guarded the old pointer-keyed cache's aliasing bug, which a content-keyed
stack of pinned fds cannot have, and holding the fds across chunks is strictly
more race-safe (a swapped ancestor resolves to the pinned original).
(cherry picked from commit 182bed6cc6b7c33e546132a069b2d5f4203cb91c)
secure_relpath_active() now hardens path resolution for ALL non-chrooted
receivers/generators, not just the non-chrooted daemon module:
!am_chrooted && (am_daemon || !am_sender) (+ daemon inner-module clause)
so a root-run or cross-user local / remote-shell transfer resolves
destination paths under the held-dirfd resolver, closing the parent-
component symlink-race / confused-deputy on the receiver side. A chroot
is its own confinement (excluded); the sender is excluded so it still
follows -L/--copy-links symlinks out of the tree.
Also confine do_rename_at()/do_link_at() each side independently: an
absolute (operator-supplied) source must not disable confinement of a
relative destination, and an absolute operator-trusted path (e.g. an
absolutized --link-dest basis) uses AT_FDCWD with the full path.
Turns green: nondaemon-symlink-race, relative-mkpath-symlink,
relative-mkpath-dir-symlink, rename-mixed-parent-transfer,
symlink-dest-backupdir. No regression (itemize/xattrs-hlink stay green
via the absolutized alt-dest basis).
Andrew Tridgell