io.c and socket.c include <poll.h> unconditionally, but configure only
required the function. A system that exposes poll() through some other
header would pass configure and then fail to compile -- the AC_CHECK_HEADERS
result for poll.h was collected and never used.
Require the header too, with its own message. Verified both ways: a normal
configure still succeeds, and forcing ac_cv_header_poll_h=no now stops with
"rsync requires <poll.h>" rather than failing later in the build.
(cherry picked from commit a0952930a1)
The MSG_IO_TIMEOUT cap and set_io_timeout()'s negative/overflow guards
were written when these loops used select(), and their comments explain
the danger as a tight select()-EINVAL spin on a negative tv_sec.
Under poll() the failure mode inverts: the timeout is a millisecond count
where a negative value means "wait forever", so a wrapped allowed_lull
hangs the process instead of spinning it. The guards are still needed and
unchanged -- only their stated reason was wrong, and a rationale that no
longer matches the code is what gets a guard removed later.
poll_timeout_ms() clamps the value too, so the guards are now belt and
braces; noted so neither looks redundant on its own.
(cherry picked from commit d5cce08503)
Follow-up to the FD_SETSIZE fix, covering the points raised in review.
Negative/overflowing I/O timeouts. set_io_timeout() could produce a negative
select_timeout (a peer-supplied MSG_IO_TIMEOUT value was applied unchecked),
and every wait now passes select_timeout * 1000 to poll(), where a negative
millisecond count means "wait forever" -- so a hostile or buggy peer could
stall the other side and bypass keepalives entirely. select() used to reject
that with EINVAL, which kept the loop and check_timeout() running. Clamp a
negative argument to 0, compute allowed_lull without overflowing near INT_MAX
(secs / 2 + secs % 2), ignore a non-positive MSG_IO_TIMEOUT value, and funnel
all three waits through poll_timeout_ms(), which keeps the count positive and
bounded.
The daemon accept loop had the same fd_set overflow. start_accept_loop() still
stored listening sockets in an fd_set, so a daemon started with enough
descriptors already open got listener fds >= FD_SETSIZE and hit the same
undefined behaviour at startup -- verified: with the old code a transfer
through such a daemon yields nothing, with this change it succeeds. Converted
it to poll() as well.
Readiness testing. Treating any non-zero revents as ordinary readiness was
wrong: poll() reports POLLERR/POLLHUP/POLLNVAL unrequested, and an invalid fd
shows up as POLLNVAL on a successful poll() rather than -1/EBADF, which left
the EBADF branches dead and let an invalid ff_forward_fd reach
forward_filesfrom_data() (where EBADF reads as EOF). Use role-specific masks
(POLL_RD_BITS / POLL_WR_BITS), handle POLLNVAL explicitly in all three loops,
and request POLLPRI so select()'s old exception set is not silently dropped.
A bidirectional fd is no longer entered twice. A direct daemon connection uses
one fd for both directions; it now occupies a single pollfd row with OR-ed
events instead of two rows carrying different masks, which also avoids the
Cygwin < 3.3.6 duplicate-entry readiness bug.
poll() is now a declared requirement: configure.ac checks for poll.h and
poll(), failing with a clear message rather than leaving it implicit.
The test no longer hardcodes FD_SETSIZE (1024 on glibc but 65536 on 64-bit
Solaris, where it would have opened too few fds and passed vacuously); it asks
the C library for the real value via a small compiled probe and skips if that
is unavailable. Its description now also covers the fortified-libc case, where
the pre-fix result is an abort rather than a hang.
(cherry picked from commit 7ef165dd45)
(cherry picked from commit bf3a11cf24)
# Conflicts:
# testsuite/highfd-hang_test.py
rsync's I/O loops (safe_read, safe_write, and the main perform_io
multiplexer) waited for readiness with select() and fd_set bitmaps. An
fd_set can only represent descriptors below FD_SETSIZE (1024 with glibc).
When rsync is started with many descriptors already open -- e.g. inherited
from a parent process that leaked fds, a high "ulimit -n", or a busy daemon
-- its own socket and pipe fds get allocated at or above 1024. FD_SET() and
FD_ISSET() then index past the end of the fixed-size fd_set, which is
undefined behavior: select() reports the fd as ready, but FD_ISSET() reads
the out-of-bounds bit as 0, so the read or write never happens and rsync
spins at 100% CPU forever with no progress. This is the long-standing
"rsync hangs at 100% CPU on large systems" report, and it matches the
MemorySanitizer use-of-uninitialized-value seen in perform_io.
Convert the three loops to poll(), which identifies descriptors by value in
a small array and has no FD_SETSIZE ceiling, so a high-numbered fd works
fine. rsync only ever waits on a handful of fds (at most three in
perform_io: in_fd, out_fd, and the files-from forward fd), so poll() is as
fast as -- or faster than -- select() here; the select()-vs-poll() cost gap
only appears when watching thousands of descriptors, which rsync never
does. The remaining select(0, ...) call is a pure timed sleep with no fds
and is unaffected.
The conversion is behavior-preserving: the same max_fd bookkeeping decides
when there is nothing to wait on, the per-fd readiness checks map to the
matching pollfd revents, and the timeout is the same (now expressed in
milliseconds).
testsuite/highfd-hang_test.py reproduces the hang deterministically by
opening enough inheritable dummy fds to push rsync's descriptors past
FD_SETSIZE before an ordinary transfer; it hangs (caught by a timeout) on
the select() code and passes instantly with poll().
(cherry picked from commit 4a751a2ceb)
(cherry picked from commit 44fdf0bc43)
Review caught a release-blocking regression in the previous commit: with
--inplace --sparse, interior zero runs inside *matching* blocks were left
allocated.
The scan I added applied only to the write path. The use_seek branch --
reached via skip_matched() when an in-place update finds identical data --
still trimmed just the leading and trailing zeros and lseek()'d over the whole
middle. Before the change, write_file() fed that data through
SPARSE_WRITE_SIZE slices, so an all-zero slice in the middle of a large
matching block became a deferred hole like any other; afterwards those blocks
stayed fully allocated. Reproduced with the reported case: an 8 MiB file whose
every 32 KiB block is 4 KiB data / 24 KiB zeros / 4 KiB data, copied onto an
identical destination with --inplace --sparse --no-whole-file
--block-size=32768, occupied 2048 KiB before this series, 8192 KiB after the
previous commit, and 2048 KiB again with this one. Content was byte-identical
throughout; only the on-disk sparseness regressed.
Rather than duplicate the scan in the use_seek branch, drop that branch and run
both cases through the one loop, with the sole difference factored into
emit_sparse_span(): a span that is not becoming a hole is written normally, or
merely seeked past when the bytes on disk already match. The hole itself is
flushed by the existing flush_sparse_hole(), which already picks do_punch_hole()
over do_lseek() while inside the preallocated extent -- and for an in-place
transfer the receiver sets preallocated_len to the basis size, so a matched
interior hole is genuinely deallocated rather than skipped over.
Verified by differential fuzzing against the pre-series binary: 37 file shapes
(including the reported one, runs either side of the SPARSE_WRITE_SIZE
threshold, all-zero and hole-free files, and randomised mixes) across both the
plain and --inplace modes, comparing contents and allocated blocks. Contents
match and allocation is never worse than before the series.
(cherry picked from commit 231de22f7c)
(cherry picked from commit bb804f80f6)
write_file()'s sparse path sliced each span into SPARSE_WRITE_SIZE (1024-byte)
pieces and write_sparse() issued one write() syscall per slice. Copying a
large *non-sparse* file with --sparse therefore cost roughly one write() per
kilobyte -- about a million write() calls for a 1 GiB file -- which on real
storage ran far slower than the same copy without --sparse (the bug report
measured 1.36 MB/s vs 391 MB/s, ~280x). The 1024-byte chunk is also smaller
than a filesystem block, so it cannot even create finer holes than a plain
copy could.
Rewrite write_sparse() to scan the whole span itself: it looks for interior
runs of zeros that are at least SPARSE_WRITE_SIZE long -- the same hole
granularity rsync has always used -- and emits each intervening non-zero
region (which may include shorter zero runs not worth a hole) with a single
write(). do_punch_hole() advances the file offset just like the lseek() path,
so flushing a deferred hole between segments keeps the position correct.
The hole granularity is unchanged, so sparseness is identical; only the
syscall pattern changes. Measured on a 100 MiB random (hole-free) file:
write() syscalls drop from 100,730 to 6,125 (~16x), now tracking the data's
natural chunking rather than its size in kilobytes. Verified byte-identical
and equally sparse output for hole-free, large-hole, small-interior-hole,
all-zero, --inplace, and --preallocate cases.
testsuite/sparse-write-count_test.py copies a 16 MiB hole-free file under
strace and asserts the write() count stays far below the old size/1024
behaviour (it skips where strace is unavailable).
(cherry picked from commit 5ecab683a8)
(cherry picked from commit 6a15079cab)
It used HAVE_MKNOD. Older Darwin has mknod() but not mknodat(), so the
call was compiled and then failed to link:
"_mknodat", referenced from:
_do_mknod_atfd in syscall.o
ld: symbol(s) not found for architecture x86_64
Reported on macOS 10.13.6 x86_64, where config.h carries
/* #undef HAVE_MKNODAT */ next to #define HAVE_MKNOD 1. The sibling
do_mknod_at() already keys off HAVE_MKNODAT and its comment names this
exact platform.
Both occurrences change together. The second guards
return -1; /* mknodat()'s errno (regular/device node) */
against an ENOSYS fallback, so leaving it on HAVE_MKNOD would report
"mknodat()'s errno" on a build where the call was never compiled -- a
stale errno from whatever ran last.
Checked by rewriting config.h the way that platform has it and compiling
syscall.c: the parent leaves one unresolved mknodat reference, this
leaves none. A FIFO still goes to mkfifoat() where that exists, and a
socket still returns EOPNOTSUPP; only the regular/device-node path
becomes ENOSYS, which is what a platform without mknodat() can offer.
(cherry picked from commit c458873481)
# Conflicts:
# syscall.c
A checked receiver-side directory option whose leaf does not exist can
be created as attacker-controlled transfer content and then consumed by
the same transfer: --backup-dir=a, with "a" arriving in-band as a
symlink pointing out of the restricted directory. The same shape works
against --copy-dest, where it reads an outside file and delivers it to
the client. Passing the leaf under the parent's /proc/self/fd pin is
not enough: the peer's symlink wins the race to the name, and rsync
creates or reads through it.
The answer is to hand rsync an inode rather than a name -- but what
inode depends on what rsync does with the option, so the policy is per
option rather than per type ("type 2" means "check when receiving", not
"is a directory"):
--backup-dir, --partial-dir rsync creates them on demand. rrsync
creates them instead, walking down from
the restricted dir one component at a
time with O_NOFOLLOW (nested names too:
make_bak_dir() builds a hierarchy), and
pins the result. The partial dir is
made 0700, as rsync makes it.
--temp-dir rsync requires it to exist, so a missing
one stays an error.
--link-dest, --compare-dest, rsync only reads through these, and a
--copy-dest missing one is the ordinary first-run
case that must keep working. rrsync
pins an empty directory it then unlinks:
the transfer behaves as with a missing
one, and there is no name left for the
peer to take over. Not quite identical:
rsync prints "--link-dest arg does not
exist" for a genuinely missing basis and
the placeholder suppresses that.
If the transfer later replaces a created name, the held inode is merely
detached -- the backup fails, it does not escape.
Without /proc/self/fd there is no way to name an inode, so on those
platforms every missing type-2 option path is refused instead -- the six
above, not the type-3 paths covered at the end.
That is the same fail-closed behaviour this change originally had
everywhere; the pinning is what buys back first use where it can.
An earlier version refused every missing type-2 leaf on every platform.
That closed the pivot but broke first-use --backup-dir and --partial-dir
-- including --partial-dir=.rsync-partial, the documented resumable-
upload idiom -- and would have broken first-run --link-dest, which is
how every rotating-snapshot script starts.
The regressions run their security assertions BEFORE their controls, so
an environment where a control fails for an unrelated reason cannot mask
the escape check by aborting first -- which is exactly what happened to
one reviewer. The alt-dest one also has to beat a race: the generator
runs ahead of the receiver, so sorting the pivot symlink first does not
guarantee it is installed before the basis lookup, and a run where the
generator won would pass vacuously. A few thousand files in between
give the receiver the head start, and the test asserts the symlink was
really installed rather than trusting the ordering. Measured 5/5 RED on
the parent, 3/3 GREEN here.
The regressions cover each branch of the policy: the backup-dir pivot,
the --copy-dest read escape (its own test), first-use --backup-dir,
nested first-use --backup-dir, first-use --partial-dir including its
mode, a refused missing --temp-dir, an accepted first-run --link-dest
with no placeholder left behind, and an existing basis still working.
Each requires the specific mechanism rather than just "the outside file
was left alone", which any unrelated failure would satisfy, and each
takes the refusal branch where the pin primitive is unavailable.
Behaviour worth knowing about, since rrsync now creates these rather
than rsync: they are created while the ARGUMENTS are parsed, so they
appear even under --dry-run, where rsync's own make_path() deliberately
does not mkdir; and one is left behind if the transfer then fails. A
pinned --partial-dir is also one directory rather than one per
destination directory, and rsync will not auto-remove it -- which is not
new, rrsync already rewrote an existing relative partial dir to its pin.
A nested --partial-dir now behaves differently from plain rsync, which
documents creating "just the last directory -- not the whole path". With
--partial-dir=a/b and "a" missing, rsync creates nothing and rrsync
creates both. Containment is unaffected -- each component is made
beneath the fd already held, O_NOFOLLOW -- but rrsync accepts a shape
rsync would not honour, and since rsync only removes the last component
of a relative partial dir, an empty parent is left behind.
Not fixed here, and NOT claimed to be: this covers the six type-2
directory options. --files-from, --log-file and receiver positional
paths are type 3; where the leaf and its parent are both missing, or on
a platform without /proc/self/fd, those still reach rsync unpinned, as
SECURITY.md describes. That is the rest of the issue, not this one.
The wrapper is handed a shim rather than RSYNC directly: RSYNC is a
multi-word command whenever the runner forces --protocol=N, and rrsync
execlp()s its RSYNC as a single executable name, so this test died
before reaching the policy under test. A fleet run caught it on the
protocol columns of three targets.
(cherry picked from commit 41f411d6e9)
The %H allow-list was wrong in both directions.
Several accepted characters change an argument's MEANING rather than its
text when they lead the value, which quoting cannot prevent because the
word stays intact -- that IS the problem:
'-' and '+' introduce options to plenty of programs; with
RSYNC_CONNECT_PROG="prog %H", hosts "-c" and "+x" arrive as
options, and "sh +x" is as real as "sh -x";
'~' is tilde-expanded by the nested shell, turning ~root into /root;
'%' is expanded by a nested fish, where %self becomes its pid.
An empty host has the same shape from the other end: it survives a direct
exec as an empty argument but disappears when a nested shell re-splits the
command, shifting everything after it. rsync://:873/m/ and ::m/ both
produce one. None of these can begin a real hostname, so refuse them in
first position only -- mid-word each is literal, which matters because an
IPv6 zone id carries its '%' mid-word.
The other direction: '+' and '~' were refused outright. They execute
nothing, and RSYNC_CONNECT_PROG exists for custom transports where %H is
often an alias the program resolves itself rather than a name the
resolver sees. Refusing them mid-word breaks that use case for no gain.
A non-ASCII host stays refused. That is a policy choice rather than a
free one: a custom connect program never calls getaddrinfo, so a Unicode
alias would otherwise work, and this does exclude it. A punycode A-label
is unaffected.
What this cannot do is bound what the named program makes of the value.
"host:-rf" arrives intact, and a program that splits on ':' may
reinterpret the tail; that boundary belongs to whoever writes the
command.
The test asserted only that a marker file was absent -- equally true when
rsync failed to parse its arguments, when socketpair_tcp is blocked, or
when touch was missing. Worse, the marker path was absolute, and a URL
authority ends at the first '/', so the injected `touch` never received
an operand and the check could not fail even with the guard gone. It now
runs with cwd set to the scratch directory and injects a bare name, so
the marker is genuinely reachable; requires the specific refusal message;
checks the exit status; and checks that ordinary hosts still arrive at
the connect program with their text intact, which is the part an
absence-only test can never show.
Fault-injected separately: dropping the guard, dropping just the
first-character check, and narrowing the set back each fail the test on
their own, in both the default and --use-tcp transports.
(cherry picked from commit dcc3c9c51a)
# Conflicts:
# testsuite/connect-prog-nested-singlequote-host-injection_test.py
The refused set was built from the characters that obviously execute
something, and missed three that a SECOND shell acts on:
'!' negates in command position. A hook written as an access check --
`pre-xfer exec = sh -c '%RSYNC_USER_NAME% false'` -- becomes
`! false`, reports success, and serves the transfer. An
authenticated user named "!" turns a denial into an approval, which
is precisely the case the fail-closed comment above exists for.
'~' is tilde-expanded, so ~root becomes /root.
'{' and '}' brace-expand in bash and zsh.
None of them execute anything on their own, which is how a set built from
the obvious metacharacters came to miss them. That is also the standing
weakness of the approach: this is a deny-list, and the two rounds of
review it took to find '!' are the argument for eventually inverting it.
The documentation is corrected with it -- it claimed every shell-active
character was refused, which this disproves -- and now lists the set.
Each listed character is pinned by the test, which needed its module
paths to EXIST first: a missing path fails the transfer on its own, so
checking the exit status alone passed whether or not the character was
refused. Removing any single character from the set now fails the test.
(cherry picked from commit f704aa4aed)
# Conflicts:
# testsuite/daemon-exec-metachar-documented-limit_test.py
# testsuite/daemon-exec-second-shell-argument-injection_test.py
Refusing a shell-active %VAR% in an exec hook is a real usability cost
and it is not confined to hostile input: the check runs on every
%RSYNC_*% value, so a module whose path holds a space cannot be
interpolated into a hook at all. `path = /srv/My Backups` with a command
mentioning %RSYNC_MODULE_PATH% refuses every transfer of that module,
with no attacker involved.
That is deliberate rather than an oversight. rsync escapes a
substitution for the quoting context it sits in, which is right for the
one shell that runs the command and wrong for a command that starts a
second one -- `sh -c '... %RSYNC_USER_NAME% ...'` hands the inner shell a
bare value, where "touc?" is glob-expanded against /usr/bin and chooses
the command rather than being data for it. Escaping for an unknown
number of passes is not possible, so the value is refused instead. Nor
can the check exempt operator-supplied values: `path` may itself be
templated from a peer one (`path = /home/%RSYNC_USER_NAME%`), and once
both are inside the same string rsync cannot tell them apart.
Say so in the manual, and give the way out: the same names are exported
to the command, so $RSYNC_MODULE_PATH inside a script is unrestricted.
The test pins both halves, since a documented behaviour with no oracle
drifts. Both are fault-injected: narrowing the refused set back makes
the interpolated half pass when the manual says it must not, and dropping
the RSYNC_MODULE_PATH export fails the workaround half.
(cherry picked from commit 5bf9940de4)
Write-only mode rejects ordinary downloads, but a remote --files-from
makes the receiving child open a server-side file and send it back over
the upload protocol. Reject server-local --files-from paths under -wo,
keeping the exact "-" sentinel that a client-local files-from upload
sends.
The control now uploads a second, unlisted source file and requires it
NOT to arrive. With a single file present an ordinary recursive upload
looks identical, so the control passed whether or not the list selected
anything.
The wrapper is handed a shim rather than RSYNC directly: RSYNC is a
multi-word command whenever the runner forces --protocol=N, and rrsync
execlp()s its RSYNC as a single executable name, so this test died
before reaching the policy under test. A fleet run caught it on the
protocol columns of three targets.
Known gap: this closes the argv route only. A per-directory merge
filter delivered over the protocol still reads a file outside the
restricted directory and returns its content in an "Unknown filter rule"
error -- reproduced with a stock client against -wo. rsync's existing
filter-file confinement does not apply because it is gated on am_daemon
and an rrsync server is not a daemon. Tracked separately.
(cherry picked from commit 576ce422fe)
parse_one_refuse_match() marked only the first long_options row whose
long name matched the configured spelling, then broke out for a
non-wildcard rule. --compress-threads and --zt are separate popt rows
that both write &do_compression_threads, so "refuse options =
compress-threads" disabled the canonical row and left the alias
accepted: the refused capability was still reachable under its other
name. The same shape covers zc/compress-choice and zl/compress-level.
An exact rule names a capability, not one spelling of it, so mark every
row that does the same thing. Comparing the raw table fields is not
enough for that: popt's `val` means different things per argInfo. For
POPT_ARG_VAL it IS the value stored in `arg`, while elsewhere a nonzero
`val` is an action code for the parser's switch, and POPT_ARG_NONE with
a destination stores 1 whatever `val` says. --del is
POPT_ARG_NONE/&delete_during/0 and --delete-during is
POPT_ARG_VAL/&delete_during/1: the same destination and the same
resulting value, but unequal as table entries, so "refuse options =
delete-during" was still evaded by --del and the mirror held too.
Compare what a row does instead -- the destination and the constant it
assigns, falling back to table-entry equality for rows that store a
runtime value or only dispatch an action. Enumerating all 258 rows,
this couples exactly one pair the field comparison missed, del and
delete-during, and changes nothing else. Opposite switches such as
--foo and --no-foo stay distinct because they assign different values.
Two regressions. The compress-threads one drives the raw daemon
protocol -- not to preserve the spelling, which -M--zt=N would do just as
well, but because it goes on to observe the worker pool the bypass
delivers. Its oracle is the refusal itself -- the alias connection torn
down and
"configured to refuse --zt" logged -- and deliberately not the resulting
worker count: an accepted --zt is a defeated refuse rule however few
threads it produces, and the daemon worker cap being added alongside
this holds that count to 9, so a count-based assertion passes while the
alias is still accepted. Run that test against the cap without this
parser fix and it does exactly that; the two changes were covering for
each other.
The delete one needs neither zstd nor a socket: --remote-option puts the
option in the daemon's argv verbatim, which is the reach an ordinary
user already has, so it drives a stock client both ways round against
modules refusing each spelling, with an unrefused module as the control.
The compress-threads test needs --use-tcp, so it skips in every other
column and is declared in the workflows that enforce a skip set, which a
fleet run otherwise reports as an unexpected skip on fourteen cells.
(cherry picked from commit c529163ef0)
# Conflicts:
# .github/workflows/almalinux-8-build.yml
# .github/workflows/cygwin-build.yml
# .github/workflows/macos-build.yml
# .github/workflows/ubuntu-22.04-build.yml
# .github/workflows/ubuntu-build.yml
A restricted dir must not let a client have the spawned rsync create
devices or special files in the served tree, but -a bundles -D into the
client's short options, so refusing -D outright breaks every ordinary
`rsync -a`. 88cee089 forced --no-D instead. That option also clears
the rdev framing, and rrsync sets it on one end only, so the file list
desynchronised: a FIFO push hung at protocol 29 and corrupted the list
at 30, and a device push failed at every protocol including 32. Use
--drop-D, which withholds the creation without touching the wire.
Only on the receiving side. A sender creates no received device or
special entry in the served tree, so there is nothing to deny and
--drop-D is a no-op there; forcing --no-D on a pull was the same
one-sided change in the other direction, and broke pulls the same way.
3.4.4 forced nothing at all and is the behaviour a pull now gets back.
rrsync-specials-denied asserted the option on a "--server --sender"
command line, which conflated the two directions. It now checks that
the receiving side forces --drop-D and still forwards the client's own
-D -- without which the two ends frame the list differently again --
that the sending side forces neither, and, rather than only what is
forwarded, that a real push cannot create a FIFO while the rest of the
transfer succeeds. An ordinary file alongside is the control, since a
push that failed outright would "deny" the FIFO too.
The device case gets its own push, because a device desynchronises at
every protocol while a FIFO only does so below 31. It needs no mknod
privilege: rsync's fake-super "%stat" xattr is what makes a file a
device to rsync, and running the RECEIVER under --fake-super too lets it
record one without privilege -- so the case asserts that nothing of that
name appears, not merely that the transfer survived.
rrsync-pull-arg-shapes could previously assert only that pulling a FIFO
did not hang, because forcing the option on that side broke the transfer
outright. It now requires the pull to succeed and deliver a FIFO, which
is what a pristine 3.4.4 rrsync does.
(cherry picked from commit 2fbb708f15)
# Conflicts:
# testsuite/rrsync-pull-arg-shapes_test.py
# testsuite/rrsync-specials-denied_test.py
-D and --no-D do two jobs at once: they decide whether devices and
special files are created, and they decide whether those entries carry
their rdev fields on the wire. send_file_entry() and recv_file_entry()
frame those fields with the same condition, but each end evaluates its
own preserve_devices/preserve_specials, so the two only agree because
both normally parse the same command line.
That makes --no-D unusable for a wrapper that controls one end of a
connection and wants to deny creation. Give it to the receiver alone
and the client's -D sender writes rdev the receiver never reads: the
file list desynchronises from that entry on. A FIFO or socket breaks
below protocol 31 -- a hang at 29, "File-list index 0 not in 0 - -1" at
30 -- and a device node breaks at EVERY protocol, current ones included,
because its arm of the condition has no protocol clause at all.
--drop-D separates the two jobs: it refuses the creation and leaves the
encoding alone. The entry is skipped through the existing non-regular
fall-through, so the visible result matches --no-D, and because it
touches no wire state it can be applied to one end by itself.
It has no effect on a sending rsync, which creates nothing.
(cherry picked from commit b607369f5f)
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 1c0bd88f0b)
# Conflicts:
# .github/workflows/cygwin-build.yml
# .github/workflows/macos-build.yml
# testsuite/rrsync-pull-arg-shapes_test.py
The HAVE_PROC_SELF_FD probe checked that readlink of a DIRECTORY's entry
returned the right path, which is not evidence of an inode pin, and two
platforms fail that assumption in opposite directions:
* NetBSD makes the entry a symlink for directories only -- readlink of
a regular file's entry fails with EINVAL -- so the probe passed and
then every pull of a file died in the post-pin check with
"post-pin readlink failed (race?): f1 Invalid argument". This is
not new: the same failure reproduces on the branch base.
* Cygwin's readlink returns the right path, but opening the magic link
RE-RESOLVES it. Renaming a directory out from under a held fd lets
the magic link reach the replacement, so the pin protected nothing
while appearing to. rrsync-sender-leaf-flip caught this as a real
outside-content leak, not as flakiness.
Only Linux provides the inode-bound magic link this depends on, so
require that explicitly and keep the runtime probes as a guard for
Linux-like environments where /proc is absent or restricted. Elsewhere
rrsync falls through to the unpinned path, as it already did on the BSDs
and macOS. proc_self_fd_pins() mirrors the rule so the race tests skip
rather than report the intended gap, and Cygwin's workflow expects both
of them to skip -- rrsync-symlink only ran there because the old probe
wrongly reported support.
(cherry picked from commit ce0f6d5a25)
# Conflicts:
# .github/workflows/cygwin-build.yml
# testsuite/rsyncfns.py
88cee089 rewrote every validated argument to /proc/self/fd/N so the
spawned rsync re-resolves it to the pinned inode. That is right for a
receiver, which open()s its destination, but a sender never opens its
source argument: send_file_list() lstat()s it first, and lstat of a
procfs magic link is always S_IFLNK. So the sender described the
argument as a symlink and sent no data -- silent data loss on
"rsync -a user@host:file dest/", the most ordinary command there is.
Of the argument shapes now covered, only a trailing-slash directory
survived.
Leonid Bugaev reported the regression, diagnosed the lstat-vs-magic-link
mechanism, and supplied the first regression test.
Which pin is usable depends on what rsync does with the argument:
* a trailing "/" or "/." directory is opened, not lstat()ed, and rsync
does follow a symlink there, so it keeps the leaf pin -- verified:
without it a flipped leaf transfers the outside directory's content;
* anything else pins one level up and passes the leaf by name. rsync
will not follow a symlink at that position (it sends the symlink
itself), -L/-k/--copy-unsafe-links are already disabled for a
restricted dir, and rsync's own leaf open is O_NOFOLLOW.
The directory pin resolves normally, including a symlink at its last
component, which is legitimate and which 3.4.4 accepts; the readlink
check afterwards is what proves the held inode is in-tree. It uses
O_PATH because reaching a known name beneath a directory needs only
search permission, and a mode 0111 parent is an ordinary way to publish
a file without letting it be listed.
Under --relative the transmitted name is the whole argument rather than
its basename, so the pin moves up to where that name starts and the rest
is spelled after a /./ marker. The client's own first marker wins if it
supplied one, including when nothing follows it; -R is parsed out of the
short-option cluster rather than sniffed for the letter, so the trailing
capability blob (-e.iLsfxC) cannot be mistaken for it.
Directory pins are keyed by (st_dev, st_ino), so a glob whose matches
share a parent inherits one descriptor rather than one per argument.
Every shape now delivers what a pristine 3.4.4 delivers, which is what
rrsync-pull-arg-shapes asserts -- it passes against 3.4.4 itself, so the
expectations are that behaviour and not this implementation's. The
"-R --no-implied-dirs" case is gated on protocol >= 30: at protocol 29
the receiver rejects it with "invalid path from sender" and transfers
nothing, which 3.4.4 does identically.
Moving the sender's pin off the leaf invalidates rrsync-symlink's oracle,
so it is reworked here rather than left failing. It patches rsync to a
stub that open()s its last argument, which is a faithful model for an
intermediate path component -- whatever rsync does with the final name,
it must not reach it through a flipped parent -- but not for the leaf: a
sender lstat()s its source and transmits a symlink there rather than
reading through it. So it now flips an intermediate directory, and the
leaf is covered against the real binary by rrsync-sender-leaf-flip, which
races both a plain file argument and a trailing-slash directory and
asserts no outside CONTENT is delivered rather than requiring a symptom
from a race that may not be won on a given run. Its trailing-slash half
is RED against a pristine 3.4.4 rrsync, which delivers outside/dir/loot.
rrsync-symlink is now sender-only. Measured over a 5s race, the stub
reached the outside marker 28 times in 98 runs as a sender and 25 in 97
as a receiver against 3.4.4; with the pin it is 0 as a sender but still
7-9 as a receiver, on the branch base as well as here. That residual is
the receiver's not-yet-existing-destination fallback, which predates this
work and is tracked in #139 rather than folded in.
Clearing FD_CLOEXEC goes through F_SETFD rather than os.set_inheritable(),
which prefers ioctl(FIONCLEX) and gets EBADF from an O_PATH descriptor on
older kernels -- every sender pull on Ubuntu 18.04 aborted with "Bad file
descriptor" the moment a directory pin was taken.
Co-authored-by: Leonid Bugaev <leonsbox@gmail.com>
(cherry picked from commit 6edb7dea2a)
# Conflicts:
# .github/workflows/macos-build.yml
# testsuite/rrsync-symlink_test.py
A pull with a local --files-from does not send the list file to the
server: it sends the literal "--files-from=-" and streams the names down
the protocol connection. 88cee089 started inode-pinning every checked
option value, so rrsync tried to realpath() and open a file named "-" in
the restricted dir and killed the connection:
post-realpath open failed (race detected): - No such file or directory
Every --files-from pull through a restricted account was broken; 3.4.4
delivers the files. Exempt the exact string "-" only, so a list file
that really is a pathname is still validated and pinned -- which the new
test's control case checks, using the command shape rrsync actually
accepts so that it reaches the pathname check rather than dying earlier
at the syntax check.
(cherry picked from commit 8678d89b2c)
auth_server() tokenised on commas AND whitespace, ignoring the documented
comma-only form, so an entry containing a space was torn in two: the rule
the administrator wrote never matched, and a rule they never wrote
appeared from its tail. For "@Group Name:deny" that means the deny is
skipped and a later :rw entry can match instead -- an authorization
bypass for a member of the denied group.
conf_strtok() already implements the documented behaviour and the
daemon's gid field already uses it (clientserver.c); this consumer was
missed when that one was fixed.
Reported by Andres Berbescu. Refs #137.
(cherry picked from commit e7986502cb)
Confining every parse_filter_file() open to the module root also caught
"filter", "include from" and "exclude from" from rsyncd.conf. Those name
operator-configured paths and pointing them outside the module -- at
/etc/rsync/excludes, say -- is the ordinary way to write them; rsyncd.conf(5)
puts no constraint on where the file lives. The result was not a refused
rule but a refused connection:
failed to open exclude file /etc/rsync/excludes:
Too many levels of symbolic links (40)
rsync error: error in file IO (code 11) at exclude.c(1582)
with no symlink involved anywhere -- just a regular file outside the module.
Mark the window in which the daemon loads its own parameters and skip the
confinement there. Everything else, in particular the peer-driven dir-merge
the leak test exercises, is still confined. Also fix the trailing whitespace
in the original hunk.
(cherry picked from commit 5eb99bb6b2)
macOS's fchmodat(..., AT_SYMLINK_NOFOLLOW) returns EPERM and applies
NOTHING when the requested setgid bit is ungrantable, so
"rsync -a --chmod=D2750,F0640" exits 23 and leaves the destination at
0755/0700 where 3.4.4 leaves 0750. fchmod() on an already-open
descriptor does the right thing: it succeeds, drops the setgid bit it
cannot grant, and applies the ordinary bits.
That fd path existed but was fenced behind "#if defined __linux__".
Guard it on O_NOFOLLOW so every platform that can open a leaf without
following a symlink uses it, and leave the fchmodat/fchmodat2 fallbacks
under __linux__. Measured on macOS with an ungrantable group:
fchmodat(2750, NOFOLLOW) EPERM, 0700 -> 0700 (dir and FIFO alike)
fchmod(fd, 2750) ok, 0700 -> 0750
fchmodat(0750, NOFOLLOW) ok, 0700 -> 0750
A FIFO observed by the lstat takes the pathname call instead, then one
retry without S_ISGID. Opening a FIFO -- even O_NONBLOCK -- makes this
process a reader for as long as the descriptor lives, which wakes a
writer blocked in open(O_WRONLY) and can cost it a SIGPIPE or the bytes
it writes before we close; the third line above is why that is
avoidable. This does not make the function FIFO-open-free: the type
comes from the lstat, so a leaf swapped to a FIFO after it is still
opened, and set_file_attrs() opens FIFOs elsewhere for ACL/xattr work.
Closing that needs the open constrained to the observed type, which is
tracked separately.
The retry is a pathname call and does not pin the inode, so a leaf
swapped for another object of the same name is chmod'd instead;
AT_SYMLINK_NOFOLLOW still keeps it off a symlink's target, and the held
parent fd still confines the ancestors, so the out-of-tree boundary is
unaffected. Only S_ISGID is retried -- clearing S_ISUID too could
discard a bit that was grantable when only setgid caused the failure.
Linux keeps the fd-first order it has always had.
The raced-to-a-symlink refusal after openat() also accepts EMLINK and
EFTYPE. ELOOP is not universal for O_NOFOLLOW on a symlink -- FreeBSD
documents EMLINK and NetBSD EFTYPE -- so those two silently skipped the
refusal and fell through to the (still symlink-safe) pathname call.
Verified on real macOS: the regression test FAILS on 93a67aa9 with rc=23
and modes 0755/0700, and PASSES here with 0750/0750.
(cherry picked from commit 5f0f8f298e)
The copy-links confinement gate null-checks module_dir but then passes
anchor to strcmp() without checking it, which the scan-build gate flags:
sender.c:291:7: warning: Null pointer passed to 1st parameter
expecting 'nonnull' [core.NonNullParamChecker]
Not reachable today -- the one caller passes module_dir -- but NULL is a
legitimate value for this parameter: secure_relative_open() reads it as
"relative to the cwd", which the else branch relies on. Only this branch
would dereference it.
(cherry picked from commit abcf37a1d3)
# Conflicts:
# sender.c
secure_walk_at() has two fast paths for the final component that call
openat(ds_cur(&ds), part, ...) directly instead of going through
ds_descend(). A final component of ".." therefore never met the anchor
floor, and what happened depended entirely on the caller's flags:
".." with O_DIRECTORY -> ELOOP (refused)
".." with O_DIRECTORY|O_NOFOLLOW -> fd for the directory ABOVE
the anchor
".." without O_DIRECTORY -> parent opened, then closed,
EISDIR returned
That was harmless while every literal ".." was rejected at the front
door, but secure_relative_open_at_beneath() now admits them and
documents the held-fd stack as refusing every climb above the anchor.
The sender's own call passes O_RDONLY|O_DIRECTORY and so was never
affected -- but the guarantee the new API advertises has to hold for
whatever flags the next caller picks.
Route a literal "." or ".." through ds_descend() before the leaf fast
paths. All three flag combinations now refuse a bare ".." with ELOOP,
and t_secure_relpath covers the matrix.
(cherry picked from commit c5bc4e3677)
# Conflicts:
# t_secure_relpath.c
Context-aware quoting is only correct for one level of shell parsing. A
hook may re-parse the substituted word in a nested shell:
pre-xfer exec = sh -c 'printf %s %RSYNC_USER_NAME% >out'
The level-1 quotes are removed before the inner shell sees the value, so
an authenticated peer's username still reaches it as syntax however
carefully it was escaped. Escaping cannot fix this; refuse instead.
A %RSYNC_*% value substituted into a shell-executed hook (early exec,
name converter, pre-/post-xfer exec) is now rejected if it holds any
character that can become shell syntax in any context: quote, backtick,
dollar, backslash, semicolon, ampersand, pipe, redirection, parenthesis,
or a control character. Word-splitting and glob characters are left
alone -- they cannot execute anything and paths legitimately contain
them. The refusal is fail-closed and logged: a hook may be an access
check, so silently skipping it is not an option.
Also fix the quote tracker itself, which moved to SHELL_SINGLE_QUOTED on
an apostrophe even inside "...", where it is an ordinary character. That
made a value in `printf %s "it's %RSYNC_USER_NAME%"` escape for the wrong
context. With the refusal above this is defence in depth, and it matters
if the refused set is ever narrowed.
The two existing hook-injection tests asserted that a metacharacter value
was quoted and the transfer still succeeded; both now expect the refusal.
(cherry picked from commit 05bd16a469)
# Conflicts:
# testsuite/daemon-exec-rsync-env-shell-escape_test.py
# testsuite/daemon-exec-singlequote-injection_test.py
Every other mutating do_*_at() wrapper (unlink, symlink, link, mknod,
rmdir, open, mkdir, rename) resolves an operator-supplied path through
owner_walk_parent() when operator_path_resolve is set. do_chmod_at()
and do_lchown_at() did not look at the flag at all, and both hand an
absolute name straight to the unconfined full-path do_chmod()/do_lchown().
set_file_attrs() is called with operator_path_resolve = 1 precisely so
that "a flipped temp-dir parent then can't redirect the chmod/chown"
(rsync.c). With no held dirfd -- an absolute --temp-dir or
--partial-dir -- both fell back to these two wrappers, so that promise
did not hold. Give them the same ownership-walk branch the others use.
S_ISLNK(mode) still takes do_chmod()'s lchmod()/setattrlist() path.
The missing branch was spotted by Omar Elsayed in review on the
partial-dir EACCES recovery PR, together with the fix approach.
Suggested-by: Omar Elsayed <omarelsayed161@gmail.com>
(cherry picked from commit 0bfcd3b0f2)
When neither AT_FDCWD nor AT_SYMLINK_NOFOLLOW is available, the function body is
a no-op warning that never reads mode or dfd, so the leading 'mode &= CHMOD_BITS'
became a dead store and dfd an unused parameter -- which the pinned clang-18
scan-build gate flags (deadcode.DeadStores). Move the mask inside the
AT_SYMLINK_NOFOLLOW guard where mode is actually used, and mark dfd/mode used in
the fallback. No behavior change on any platform that has the symlink-safe
primitive.
(cherry picked from commit 7b16872eff)
The daemon rejection for a peer-selected FNAMECMP_PARTIAL_DIR basis that the
confined open declined fired at every protocol. In-place partial updates are
only negotiated at protocol 30+ (CF_INPLACE_PARTIAL_DIR); at protocol 29 no
partial-basis redirect is possible, and the receiver already handled such a
transfer safely by completing it with no basis. Gate the abort on
inplace_partial so a pre-30 daemon falls back to the safe no-basis path instead
of aborting a legitimate transfer with a protocol error.
Fixes operator-path-partial-dir-daemon at protocol 29.
read_ndx_and_attrs() sanitized the wire-supplied xname (the alternate-basis
leaf name sent with ITEM_XNAME_FOLLOWS) only when sanitize_paths was set,
which is the daemon side. A client receiver has sanitize_paths == 0, so a
malicious server could send an xname containing ".." and, joined to an
operator basedir (--link-dest / --compare-dest / --copy-dest, or the fuzzy
dir), have the client open an out-of-tree file as the delta basis -- a
client-side arbitrary-read / file-existence-oracle / FIFO-hang. The ownership
walk in secure_basis_open() does not stop this: it deliberately follows a
plain ".." to a regular file (the legitimate --link-dest=../01 sibling, #915)
and only refuses foreign-owned symlink components.
Sanitize xname unconditionally. The operator basedir may legitimately be
relative, but the leaf name that arrives over the wire never legitimately
needs ".." or a leading "/".
Reported by z3r0s.
A daemon serving a writable, non-chrooted module reads a client-requested
--files-from=:LIST through open_no_attacker_symlinks(), which follows a
symlink owned by uid 0 or the euid. The module-root confinement in that
resolver (abspath_excluded_by_module) only fires when operator_path_resolve
is set, and this open left it clear -- so a trusted-owned symlink whose
target escapes the module was followed.
An attacker can obtain such a symlink without owning it: a --backup-dir push
makes the daemon back up the old destination symlink with the daemon's own
(root) ownership, and a parent-swap race can leave that root-owned backup
symlink pointing outside the module. A later --files-from=:backup/... then
reads out-of-module file content as the file list, bypassing the same-uid
ownership constraint that normally protects files-from.
Set operator_path_resolve around the files-from open so the ownership walk
also refuses a trusted-owned symlink that redirects the list outside the
module root. A daemon has no rsyncd.conf "files from" of its own, so this
path is always client-requested and confining it is unconditional. No-op off
a daemon (the module-root check only fires when am_daemon).
The same ownership-walk opener backs the daemon merge/--exclude-from reads in
exclude.c, but those also load the module's own "include from"/"exclude from"
admin files, which on a non-chrooted module may legitimately live outside the
module; confining them there needs a client-vs-admin distinction and is left
to a separate change.
io_timeout can reach INT_MAX -- from an operator --timeout (options.c parses it
as a plain int, unbounded and even negative) or a peer's MSG_IO_TIMEOUT (now
also capped at 86400 in read_a_msg). Several signed computations then misbehave:
* allowed_lull = (io_timeout + 1) / 2 overflows to a negative allowed_lull /
select_timeout; select() then returns EINVAL on the negative tv_sec, which
isn't EBADF, so the read loop spins at 100% CPU forever (io_timeout ~= 68
years never fires check_timeout), plus a keepalive flood. Compute
ceil(io_timeout/2) in a wider type so "+ 1" cannot overflow.
* the generator and sender derive an int loop-check limit as allowed_lull * 5
(generator.c, sender.c), which overflows for a large allowed_lull. Cap
allowed_lull so that product stays in range -- invisible to real use, as
allowed_lull is the keep-alive half-interval and INT_MAX/5 seconds is over
13 years.
* a negative --timeout drove allowed_lull / select_timeout negative the same
way; treat secs < 0 as "no timeout" up front.
The overflows are undefined behaviour, so plain -O2 gcc/clang happen to keep
select_timeout at 60, but -fwrapv / -fno-strict-overflow (common hardening) wrap
to the spin and -ftrapv aborts.
Reported by z3r0s.
A malicious server can send MSG_IO_TIMEOUT with val near INT_MAX
(0x7FFFFFFF). The existing val <= 0 guard prevents timeout disabling,
but a large positive value passes through to set_io_timeout() where
(io_timeout + 1) / 2 overflows signed int, wrapping allowed_lull and
select_timeout negative. Every subsequent select() returns EINVAL
immediately, trapping the client in a tight CPU loop.
Cap the accepted timeout at 86400 seconds (24 hours), which is well
above any practical timeout and avoids the overflow in set_io_timeout().
Reported-by: z3r0s <https://github.com/z3r0s6>
hash_search() walks the entire hash-table chain for the current rolling
checksum at every byte offset of the source file. Disk and VM images
contain large runs of identical blocks, so a single weak checksum
(get_checksum1) can collide thousands of times and pile every one of
those blocks onto one chain. When the sender then rolls across a region
whose weak checksum keeps landing on that chain without ever producing a
strong-checksum match, it re-walks the whole chain for every byte, giving
O(file_size * chain_length) behaviour. The result is rsync sitting at
100% CPU for hours with no apparent progress -- the long-standing "rsync
hangs on large files" reports.
Cap the number of same-weak-checksum candidates examined per offset at
MAX_CHAIN_LEN. Once the cap is hit we treat the offset as a non-match and
roll forward a byte; any block skipped this way is simply sent as literal
data, so the transferred result is always correct -- only the transfer
size is marginally affected. This is purely a sender-side search limit:
it changes no checksum, emitted byte, or protocol field, so a capped
sender interoperates with an unmodified receiver and vice versa.
On a synthetic 40000-block basis sharing one weak checksum, syncing a
60KB source dropped from ~18.4s to ~0.7s; the unbounded cost grows with
the square of the file size.
testsuite/hashsearch-chain_test.py reproduces the pathology with a tiny
basis of weak-checksum-colliding decoy blocks and asserts, via the
existing false_alarms counter (--debug=deltasum1), that the per-hash-hit
chain walk stays bounded. The assertion is exact and machine-independent
rather than timing-based.
parse_chmod()'s 'a' clause set the "where" bits but not topbits, so "a+s" fell
through to setuid only and dropped setgid (chmod(1) sets both). Add
S_ISUID|S_ISGID to topbits for 'a'.
Reported-by: Leonid Bugaev
(cherry picked from commit 0d0e505902)
logit() wrote the message to the log file with a raw fprintf, so an
attacker-controlled filename could inject terminal escapes that an admin later
executes when cat'ing the log (CWE-117). Route it through filtered_fwrite(),
keeping the trailing newline raw. Also escape C1 controls (0x80-0x9f, incl CSI)
on filtered_fwrite's use_isprint=0 path, which previously caught only C0.
Reported-by: Leonid Bugaev
The C1 escaping is gated by an escape_c1 flag set only for the log path, so
--8-bit-output / iconv terminal output still passes 8-bit bytes (incl. UTF-8)
through unchanged.
(cherry picked from commit ad64e99590)
do_symlink_at() and do_rmdir_at() were the only operator-path syscall
wrappers still missing the operator_path_resolve branch that
do_mknod_at()/do_open_at()/do_rename_at()/do_unlink_at() already carry:
an absolute operator path (e.g. an absolute --backup-dir) took the
'*path == "/"' arm straight to the bare do_symlink()/rmdir(), whose
libc path resolution follows a parent-component symlink. A local
attacker who owns a parent component of the backup tree could thus
redirect a backup symlink creation or a backup-dir rmdir outside the
intended tree.
Route both through owner_walk_parent() + symlinkat()/unlinkat() when
operator_path_resolve is set, mirroring the existing wrappers, so a
foreign-owned parent component is refused while the operator's own is
followed. symlink_optout_allowed() (--insecure-links / 'insecure
links =') restores the legacy following.
Tests: operator-path-backup-symlink and operator-path-backup-rmdir
drive a live parent-component swap (native C flipper) against a local
--backup-dir push and confirm nothing escapes the backup tree (RED
before, GREEN after).
Co-authored-by: Andrew Tridgell <andrew@tridgell.net>
log_formatted() renders %% as a literal '%', but log_format_has() still
rescanned the literal '%' as the start of a new escape, so a format such
as --out-format='%%i' misdetected the 'i' and turned on itemizing (and
'%%b'/'%%c'/'%%C' likewise perturbed log_before_transfer and checksum
retention). Skip the literal so both parsers agree.
Extends the ki58 test: an attribute-only change must not be logged under
--out-format='%%i %n', while a transferred file still renders the
literal '%i'.
Andrew Tridgell