Both callers pass non-null from/to, but the 'to &&' test taught the
clang analyzer that to may be null, and it then walked a null to
through copy_file -> unlink_and_reopen -> robust_unlink into glibc's
nonnull-annotated strlcpy, failing the scan-build gate. The args are
required non-null, so test the first byte directly.
(cherry picked from commit c53d107f97)
The cross-filesystem fallback copied to the dest and unlinked the source without
operator-path confinement, so an absolute --temp-dir/--partial-dir on another
filesystem was opened/unlinked via plain libc -- a raced parent symlink could
redirect the dest-write or source-unlink out of the module. The source READ was
already confined; flip operator_path_resolve for an absolute (operator) path
around the copy_file dest-open and the do_unlink_at, leaving relative in-module
paths on the secure_relative_open arm.
This is the EXDEV-fallback backstop for the same absolute --partial-dir /
foreign-owned-parent-symlink escape that operator-path-partial-dir_test.py
already exercises at the handle_partial_dir() layer (which confines the staging
dir before this code runs). A dedicated test for the EXDEV copy_file path itself
would need the main tmp->final rename to fail first and then hit EXDEV on a
foreign-owned raced parent -- a nested, timing-dependent trigger.
Reported-by: Leonid Bugaev
(cherry picked from commit fd4c75116b)
After the backward walk, s points at the first char of the prior component and
s[-1] is its leading '/', so the boundary test must read s[-1] (not *s) and t
must reset to s (not s+1). The old off-by-one left CFN_COLLAPSE_DOT_DOT_DIRS
dead for all multi-component and absolute paths. The peer-traversal guard
CFN_REFUSE_DOT_DOT_DIRS is checked first and is unaffected, so this is a
normalization-correctness fix, not a traversal hole.
Reported-by: Leonid Bugaev
(cherry picked from commit fbeb553b73)
The hardened receiver confined the destination side of an xattr/ACL copy (the
fsetxattr/acl_set_fd through a held O_NOFOLLOW fd) but still read the SOURCE side
by path: copy_xattrs() did get_xattr_names/get_xattr_data on the source path, and
make_backup() cached the backed-up file's ACL/xattr via get_acl()/get_xattr() by
path. A local module writer could race the source/basis parent to a symlink
after the confined content/stat open and before that path-based metadata read,
so out-of-module xattrs/ACLs got copied onto an in-module destination or backup.
Thread a source fd through the read side, mirroring the existing dest-fd plumbing:
- get_xattr_data(), get_xattr() and get_xattr_acl() gain an fd arg (get_xattr_names
already had one) and use sys_fgetxattr/sys_flistxattr when fd >= 0; this also
covers the --fake-super ACL-as-xattr read in get_rsync_acl().
- copy_xattrs() gains a source_fd; copy_file() passes its held source fd (ifd)
and keeps it open across the xattr copy (closing it on the fsync error path
too); gen_entry_copy_xattrs() O_NOFOLLOW-opens the basis leaf under the
confined resolver (with O_DIRECTORY for a directory basis) and passes it.
- make_backup() pins the source leaf with a confined O_NOFOLLOW fd
(backup_source_fd, like set_file_attrs's op_leaf_fd) and reads its ACL via
get_acl_fdat() and its xattrs via get_xattr(fd); the in-place delta-backup in
the generator pins fname the same way. On a hardened receiver a raced/absent
leaf skips the cache rather than reading through a flippable path.
Non-hardened receivers (fd < 0) keep the path-based behaviour unchanged. The
basis COMPARE reads (the generator deciding a match) stay path-based: they never
copy out-of-module metadata onto a file, so they are not part of this sink.
The opt-out is meant to restore stock-3.2.7 "follow existing symlinks in the
module" behaviour, but several daemon symlink-resolution sites called the
confined resolver unconditionally and never consulted symlink_optout_allowed(),
so a module with "insecure links = yes" still refused to follow a symlinked
directory (change_dir ELOOP) or an alt-dest basis. Gate every such site on the
opt-out -- follow plainly like 3.2.7 when set, confine otherwise:
- change_dir() relative daemon branch (util1.c): plain chdir() under the
opt-out instead of secure_relative_open(), so a peer can read a path through
a symlinked directory again.
- basis_link_stat() (generator.c): the chrooted-relative and non-chroot
branches honour the opt-out. The non-chroot ABSOLUTE branch (a basis rooted
under the module by check_alt_basis_dirs, which can reach an in-module
symlink) is, when NOT opted out, resolved through owner_walk_parent so a
target landing outside the module root is refused -- closing a confirmed
--compare-dest=/symlink out-of-module read oracle. The leaf is taken
O_NOFOLLOW under the confined parent so --copy-links can't follow a leaf
symlink out; --fake-super folds its %stat via the held fd. A relative
sibling basis (--link-dest=../01) keeps the plain path (#915/#930).
- secure_basis_open() (receiver.c): plain do_open() under the opt-out.
- use_secure_symlinks (clientserver.c): cleared under the opt-out, so the
receiver's protected-regular EACCES write fallback is legacy too.
- secure_sender_parent_fd() (sender.c): declines (errno=0) under the opt-out
so --remove-source-files re-stats the plain path.
Default modules (insecure links = no) are unchanged and stay fully confined;
the opt-out is per-module and a client cannot enable it.
copy_file() routed a RELATIVE source through secure_relative_open (parents
confined) but opened an ABSOLUTE source -- an operator basis such as an absolute
--copy-dest -- with bare do_open_nofollow, which refuses only a leaf symlink and
follows every parent. basis_link_stat() refuses a foreign-owned basis at stat
time, but a parent flipped to a foreign symlink between that stat and this open
redirects the basis read out of tree (an out-of-tree content read-leak into the
destination; RED on 3.4.x, GREEN here).
Resolve an absolute source's parents through owner_walk_parent (foreign-owned
parent symlink refused, operator's own dirs/uid0/euid symlinks followed).
operator_path_resolve is set only across the walk -- so module-exclude is
enforced -- and restored, leaving the caller's value for the dest side; that is
why confining the source here does not re-open the copy_xattrs dest race that
wrapping the whole copy_altdest_file would (copy-xattrs-symlink-race stays green).
(Leonid Bugaev May-2026 re-audit, KI-46.)
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.
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)
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)
- 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.
- 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 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)
Preparation for broadening the symlink-race resolver to non-daemon
receivers: a --link-dest/--copy-dest/--compare-dest dir is operator-
supplied and legitimately outside the destination tree (e.g.
--copy-dest=../to), so the confined resolver must not reject its "..".
- check_alt_basis_dirs() now absolutizes a relative basis dir against the
destination curr_dir for local (!sanitize_paths) transfers too, not just
dry_run>1. An absolute path makes the do_*_at() wrappers and copy_file()
use plain (operator-trusted) resolution.
- copy_file() routes the source open through the secure resolver only for a
*relative* source under the broadened gate; an absolute (absolutized)
basis source uses do_open_nofollow().
set_times_at() is the held-dirfd form of set_times() (utimensat tier;
returns -2 where the active tier has no dfd form so the caller falls back).
make_path() now creates its parent chain through do_mkdir_at(), which
resolves the parent via the secure resolver, closing the --relative
implied-parent symlink escape once the gate covers the receiver.
Commit d046525d made my_alloc() calloc every fresh allocation and made
expand_item_list() memset the freshly grown tail, to hand out predictably
zeroed memory. But that forces the kernel to back pages callers never
touch: each per-directory file_list pre-allocates a FLIST_START-entry
(32768) pointer array -- 256KB -- and calloc now zeroes the whole array
even for an empty directory. With incremental recursion over many
directories the resident set explodes; 80000 empty dirs went from ~336MB
to ~10.8GB.
Restore the pre-d046525d malloc/calloc split: fresh allocations use
malloc (so untouched tails stay lazy) and only explicit do_calloc
requests (new_array0) are zeroed. Callers that need zeroed memory
already ask for it, and the full test suite passes.
Thanks to @guilherme-puida for the report (#959).
Fixes: #959
(cherry picked from commit 4bfd18d195)
The symlink-race hardening routed the receiver's basis open through
secure_relative_open(), which rejects any '..' -- so a sibling
--link-dest=../01 on a use-chroot=no daemon was silently ignored and every file
re-transferred (#915/#928, a regression from 3.4.1).
Narrow the confinement to the sanitizing daemon (am_daemon && !am_chrooted) and
re-anchor it at the module root, the real trust boundary: secure_relative_open()
prefixes the cwd's module-relative path (from rsync's logical curr_dir[], a
guaranteed lexical prefix of module_dir) and resolves beneath module_dir, so
RESOLVE_BENEATH permits an in-module '..' climb while still rejecting one that
escapes the module. secure_basis_open() opens with a bare do_open() in the
non-sanitizing cases. t_stub.c gains weak curr_dir[]/curr_dir_len for the
helpers (via #pragma weak on non-GNU compilers, where rsync.h erases
__attribute__).
Two tests: link-dest-relative-basis asserts the in-module '..' is honoured;
link-dest-module-escape asserts a --link-dest=../../OUTSIDE climb that leaves
the module is refused (not hard-linked to an outside file). See upstream
PR #930.
Thanks to @fufu65 (#915) and @JetAppsClark (#928) for the reports.
(cherry picked from commit 948edffb43)
Three related codex audit findings:
Finding 3a: copy_file()'s source open in util1.c used
do_open_nofollow(), which only rejects a final-component
symlink. A parent-component symlink (e.g. --copy-dest=cd where
cd -> /outside) follows freely and reads outside the module.
Route through secure_relative_open() with O_NOFOLLOW.
Finding 3b: generator.c's in-place backup-file create still
used a bare do_open with O_CREAT, leaving a tiny but reachable
parent-symlink window between the secure unlink (already
through do_unlink_at) and the create. Add do_open_at() that
goes through a secure parent dirfd, and route the call site
through it.
Finding 3c: copy_file()'s destination open in
unlink_and_reopen() had the same bare-do_open pattern; route
through do_open_at as well.
Adds testsuite/copy-dest-source-symlink.test and
testsuite/bare-do-open-symlink-race.test as regression coverage
for both attack shapes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the rest of the path-based syscall wrappers and migrate every
receiver-side caller:
- do_lchown_at, do_rename_at, do_mkdir_at, do_symlink_at,
do_mknod_at, do_link_at, do_unlink_at, do_rmdir_at,
do_utimensat_at, do_stat_at, do_lstat_at
Same shape as do_chmod_at: open each parent under
secure_relative_open(), call the *at() variant against the dirfd,
fall through to the bare path-based syscall in non-daemon /
chrooted / absolute-path / no-parent cases. macOS's
setattrlist-based set_times tier is also routed through the
utimensat_at path on daemon-no-chroot.
Hardenings to secure_relative_open() itself:
- confine basedir resolution under the same kernel mechanism
used for relpath (basedirs from --copy-dest / --link-dest are
sender-controllable in daemon mode)
- reject any '..' component (bare '..', 'foo/..', 'subdir/..')
so the per-component O_NOFOLLOW fallback can't escape
- return the dirfd we built up from the per-component fallback
when the caller passed O_DIRECTORY (otherwise every do_*_at
failed with EINVAL on platforms without RESOLVE_BENEATH)
Adds testsuite/alt-dest-symlink-race.test and
testsuite/secure-relpath-validation.test (with t_secure_relpath
helper) as regression coverage for the new hardenings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The receiver's chdir(2) into a destination subdirectory followed
attacker-planted symlinks at every path component. Once CWD
escaped the module, every subsequent path-relative syscall (open,
chmod, lchown, ...) inherited the escape -- defeating
secure_relative_open's RESOLVE_BENEATH anchor against AT_FDCWD,
since the anchor itself was now outside the module.
Route change_dir's relative target through secure_relative_open()
and fchdir() to the resulting dirfd in am_daemon && !am_chrooted
mode, so the chdir step itself can no longer follow a parent-
symlink. Same treatment applied to the CD_SKIP_CHDIR /
set_path_only path so it also can't follow attacker symlinks
during path tracking.
Adds testsuite/sender-flist-symlink-leak.test covering the
sender-side flist resolution variant of the same primitive.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Change my_alloc() to use calloc instead of malloc so all fresh
allocations return zeroed memory. Also zero the expanded portion
in expand_item_list() after realloc, since it knows both old and
new sizes. This gives more predictable behaviour in case of bugs
where uninitialised or stale memory is accidentally accessed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
when we open a file that we don't expect to be a symlink use
O_NOFOLLOW to prevent a race condition where an attacker could change
a file between being a normal file and a symlink
When using the --fuzzy option to try and find close matches locally,
the edit distance algorithm used is O(N^2), which can get painful on
CPU constrained systems when working in folders with tens of thousands
of files in it.
The lower bound on the calculated Levenshtein distance is the difference
of the two strings being compared, so if that difference is larger than
the current best match, the calculation of the exact edit distance between
the two strings can be skipped.
Testing on the OpenSUSE package repo has shown a 50% reduction in the CPU time
required to plan the rsync transaction.
* add tests to exercise copy_file
* Extract new function unlink_and_reopen from copy_file
The argument `ofd` to copy_file is always set to -1 unless
`open_tmpfile()` is called at generator.c:909
This change
* removes assignment to a function argument
* renames argument `ofd` to `tmpfilefd` in line with existing uses
* extracts a new function `unlink_and_reopen` which is static to util1.c
* rewrites header comments for copy_file
Andrew Tridgell