The exclude-self rule that a ":e" merge synthesizes is built by hand with
new0(), so it inherited no flags. While the merge file was still being
parsed the global parse state masked that, but once parsing finished the
stored rule looked argument-origin, and report_filter_result() printed its
pattern -- a merge file's own text -- verbatim:
[sender] hiding file PAT-x9 because of pattern PAT-[x]9 [per-dir ...]
Plain -vv reaches this on a stock client; no --debug is involved. That is
the fifth site of this shape, and the first to get there by constructing a
rule rather than by printing one, so the redaction helper could not catch it.
Also fix the location a per-directory merge reports. Its fname points into
dirbuf, which is cut back to the directory before the name was saved, so the
error said "<rule from .../src/ line 1>" instead of naming .rsync-filter --
no leak, but it breaks the "redact what, keep where" bargain the rest of this
work depends on. Save the name before the truncation.
The rrsync test's claim to close "the rest of the FILTER trace family" was
too strong and is corrected: options.c maps verbosity onto the debug flags,
so -vvv still raises a restricted server to FILTER2 and its trace metadata
comes back. Rule text stays redacted at every verbosity, which is the
property that matters; -vvv is added to the unaffected-transfer cases.
A filter rule that fails to parse was printed back verbatim. When the
rule came from a file rather than an argument, that text is file
CONTENT, and the peer picks which file gets merged: a per-directory
merge rule travels over the protocol, so no argument of ours ever names
it and nothing a wrapper can see mentions it either. Any line that is
not valid filter syntax therefore came straight back to the peer -- a
read-any-line oracle over an rrsync restricted account or a daemon
module, neither of which confines the merge open.
The syntax errors turned out to be the smaller half. The MATCH trace
names the pattern that acted, and report_filter_result() logs at level 1
for a sender or generator, so plain -vv -- no --debug, nothing a stock
client cannot send -- returns a server-side merge file's rules:
[generator] protecting file X because of pattern <the file's text>
So provenance is carried on the rule itself (FILTRULE_FROM_FILE), not
just in the parser: a deferred ":" merge is processed long after the
file that named it was read, and its own name is file content too.
TEXT_FROM_FILE() consults the parse-time context and the rule, so both
the immediate and the deferred paths redact.
Rather than test the provenance at each message -- which is how the last
few of these were found, one at a time, after the ones before them were
fixed -- every string that is or is built from a rule's own text goes
through rule_text(). It returns the text for an argument-supplied rule
and a description of where it came from otherwise, so a message added
later cannot reintroduce the leak by forgetting to check, and there is
one place to audit. rule_detail() does the same for the extra detail a
message adds ABOUT the text: a character of it, an offset into it, the
[not found] bit.
Thirteen sites now route through them: the syntax errors; the modifier
character (one byte of the file, a slower oracle but still one); the
failed-open and merge-depth messages, whose pathname is file content
whenever a rule named it -- and errno with them, since it answers "does
this path exist"; both over-long messages, the deferred one of which
needed no verbosity at all; both merge-name overflows; the long-named
directory error; the [not found] openability bit; the match trace; the
add_rule, parse_filter_file and daemon-hidden traces; and the per-dir
mergelist label, which had the name baked in.
rule_detail() covers more than it first looks: the trailing-whitespace
CAUTION is computed from the rule's last byte, and "hidden by daemon
filter" distinguishes a daemon-filter rejection from an ordinary open,
so both would answer questions about text the peer cannot see.
The regression proves the chokepoint rather than the sites: making
rule_text() return its input unconditionally fails the test. It also
pins what must NOT change for the user's own rules -- the whitespace
warning still fires, and an over-long argument rule is still reported at
full length (the helper buffers at BIGPATHBUFLEN, as rprintf does, so
redaction does not quietly truncate what the user typed).
Bounded and left alone: the numeric rflags in the FILTER2 trace and the
in/exclude wording still describe a file-derived rule without quoting
it, and the daemon's own FLOG line records the name it filtered -- that
one goes to the operator's log, not the peer.
Rules given AS arguments are still echoed in full -- that text is the
user's own, and hiding it would only make ordinary typos harder to fix.
Where a rule did come from a file, the diagnostic names the file and
line instead, which is more useful anyway.
Two things the location itself needed: fname can point into
parse_merge_name()'s static buffer, which a merge rule inside the same
file overwrites while we are still reading it, so a rule after a nested
merge was blamed on the nested file -- keep our own copy. And a CRLF
pair was counted as two line endings while word-split mode counted
tokens rather than lines, so the number pointed at nothing; consume the
LF of a CRLF (preserving the byte for the next rule if pushback ever
fails), and report word-split sources without a line number.
Not covered, deliberately: a rule's provenance is not serialized by
send_filter_list(), so it does not survive to the far side. That is
right -- only the client sends that list, and the server already knows
the patterns the peer gave it.
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.
The CLEAR_LIST guard in parse_rule_tok checked rule->rflags for
FILTRULE_NO_PREFIXES, but NO_PREFIXES is a template-level flag
excluded from FILTRULES_FROM_CONTAINER inheritance. The guard
was always true for CVS rules, causing RERR_SYNTAX abort instead
of clearing the list.
Fix: check template->rflags instead of rule->rflags.
Regression test: testsuite/ki73-cvs-clear-list_test.py
Refuse malformed/hostile wire input that could crash or corrupt the receiver:
io.c (out-of-range file index, count*blength OFF_T overflow, read_args NUL room,
deferred in_multiplexed), flist.c (sub-flist after the final flist is freed,
FLAG_HLINKED on dirs / gated on preserve_hard_links, parent_ndx bound, cleared-
slot ndx in the transfer phase), hlink.c (undeclared cross-flist gnum -> error not
assert), match.c (clamp peer flength, re-check len before want_i), log.c (drop
peer-reachable asserts and F_SUM deref), exclude.c (merge-file recursion cap,
trailing-backslash heap fix), lib/pool_alloc.c (ASan redzone for pool underflow).
Glibc 2.43 added C23 const-preserving overloads to various string functions,
which change the return type depending on the constness of the argument(s).
Currently this leads to warnings from calls to strtok() or strchr().
Fix this by properly declaring the respective variable types.
Signed-off-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Fortified (-D_FORTIFY_SOURCE=2 for gcc) builds make strlcpy() crash when
its third parameter (size) is larger than the buffer:
$ rsync -FFXHav '--filter=merge global-rsync-filter' Align-37-43/ xxx
sending incremental file list
*** buffer overflow detected ***: terminated
It's in the exclude code in setup_merge_file():
strlcpy(y, save, MAXPATHLEN);
Note the 'y' pointer was incremented, so it no longer points to memory
with MAXPATHLEN "owned" bytes.
Fix it by remembering the number of copied bytes into the 'save' buffer
and use that instead of MAXPATHLEN which is clearly incorrect.
Fixes#511.
- Avoid implied rules on generator and (with extra certainty) on server
- Add -R implied-directory path elements as directory includes
- Log about extra file-list checking using a new --debug=FILTER3 level
- All the memory-allocation macros now auto-check for failure and exit
with a failure message that incudes the caller's file and lineno
info. This includes strdup().
- Added the `--max-alloc=SIZE` option to be able to override the memory
allocator's sanity-check limit. It defaults to 1G (as before).
Fixes bugzilla bug 12769.
- Change default_cvsignore char[] into a define.
- Make the DEFAULT_DONT_COMPRESS and DEFAULT_CVSIGNORE defines get set
based on their info in rsync.1.md.
- Add a few more don't-compress suffixes from Simon Matter.
Andrew Tridgell