mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-09-14 14:18:23 -04:00
Samuel Henrique
c529eccff7
* options: accept --max-alloc=0 again, resolved to the parser's own ceiling
3.5.0 rejected --max-alloc=0 (CVE-2026-53794). The stated rationale was that a
zero cap "disabled the per-allocation size cap (the defense behind
CVE-2024-12084)". That was true up to 3.2.7, where the check short-circuited:
if (max_alloc && num >= max_alloc/size)
but 2f9b963a ("Make `--max-alloc=0` safer", 3.3.0) removed the short-circuit and
mapped 0 to SIZE_MAX at parse time, leaving the guard unconditional. Since the
guard admits an allocation only when num < max_alloc/size, num*size stays below
max_alloc at every setting, so the num*size overflow check was armed for 0 just
as for any other value. From 3.3.0 onward, 0 raised the magnitude ceiling and
nothing else -- and an explicit 8191P raises it exactly as far, is accepted, and
is forwarded to the peer, so rejecting 0 removed no capability.
What it did remove is the only portable spelling. The parser's ceiling is
SIZE_MAX/2, so it tracks the build's word size: on ILP32 the suffix multiplier
alone exceeds the bound, making every P and T value an error whatever the digits
and capping the option at 2047M. Because max_alloc_arg goes on the wire
un-normalized, a 0 was re-resolved by each side against its own SIZE_MAX; any
literal is resolved once on the client and shipped verbatim, so nothing above
2047M survives a 64-bit client talking to a 32-bit daemon. There is no number a
user can compute that does what 0 did, which is what #1056 ran into.
So accept 0 again, but resolve it to SIZE_ARG_MAX (SIZE_MAX/2) rather than
SIZE_MAX, so it lands exactly on the largest value that could also be typed and
is no longer a limit only the 0 spelling can reach. Keep forwarding it
verbatim: that per-side resolution is the property worth having. This leaves
the substantive half of the 3.5.0 hardening -- bounding parse_size_arg() against
the unbounded `size *= atof(size_arg)` -- untouched.
The residual concern, a 0 forwarded to a <= 3.2.7 daemon that honours it, is not
something a client-side check can address: the client is the attacker's own
code, as daemon-max-alloc-zero_test.py noted in its own docstring. Operators
who want peers kept off their cap have `refuse options = max-alloc`.
Also drops the now-unreachable rejection message, which left the min-value error
recommending a value the parser refused ("min: 1.00M or 0 for unlimited").
Tests: max-alloc-zero replaces max-alloc-zero-rejected and
daemon-max-alloc-zero, whose assertions are the behaviour being reverted. It
checks that 0 is accepted, that it reaches the peer as the literal "0" rather
than a resolved number (verified against a negative control that normalizes it),
and that the parser's upper bound still rejects an out-of-range value.
Fixes #1056
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* testsuite: refresh Cygwin expectations
* docs: note max-alloc safety
89 lines
3.4 KiB
Python
Executable File
89 lines
3.4 KiB
Python
Executable File
#!/usr/bin/env python3
|
|
"""``--max-alloc=0`` means "the largest limit this build supports".
|
|
|
|
Three things are asserted, and the second is the reason 0 is worth keeping as a
|
|
spelling at all:
|
|
|
|
1. 0 is accepted, and a transfer using it works.
|
|
|
|
2. 0 reaches the peer as the literal "0", not as a resolved number. Each side
|
|
then resolves it against its own SIZE_MAX. That is what makes 0 the only
|
|
value correct for both ends of a mixed-word-size pairing: the ceiling is
|
|
SIZE_MAX/2, so any number large enough to be worth setting on a 64-bit
|
|
client (over 2047M) is refused as "too large" by a 32-bit daemon.
|
|
|
|
3. The parser's upper bound is still enforced. Accepting 0 again must not
|
|
bring back the unbounded ``size *= atof(size_arg)`` that was fixed in
|
|
3.5.0, so an out-of-range value is still rejected rather than wrapping.
|
|
|
|
The forwarding check in (2) deliberately inspects the argv the remote shell is
|
|
handed rather than a transfer outcome: a resolved number also copies files
|
|
happily on a same-word-size pair, so an outcome-based assertion would pass on
|
|
exactly the configuration this behaviour does not matter for.
|
|
"""
|
|
|
|
import os
|
|
import shlex
|
|
|
|
from rsyncfns import (
|
|
SCRATCHDIR, SRCDIR, expect_fail, rsh_cmd, rmtree, rsync_argv,
|
|
rsync_path_arg, run_rsync, test_fail,
|
|
)
|
|
|
|
base = SCRATCHDIR / 'max-alloc-zero'
|
|
rmtree(base)
|
|
src = base / 'from'
|
|
dst = base / 'to'
|
|
src.mkdir(parents=True)
|
|
dst.mkdir(parents=True)
|
|
(src / 'file.txt').write_text('hello\n')
|
|
|
|
# --- 1. 0 is accepted -------------------------------------------------------
|
|
|
|
run_rsync('-r', '--max-alloc=0', f'{src}/', f'{dst}/')
|
|
if (dst / 'file.txt').read_text() != 'hello\n':
|
|
test_fail('--max-alloc=0 did not copy the file')
|
|
|
|
# --- 2. 0 goes on the wire un-normalized ------------------------------------
|
|
|
|
argv_log = base / 'server-argv'
|
|
wrapper = base / 'log-rsh.sh'
|
|
wrapper.write_text(
|
|
'#!/bin/sh\n'
|
|
'# Log the command line built for the peer, then behave like lsh.sh.\n'
|
|
f'printf \'%s\\n\' "$*" >> {shlex.quote(str(argv_log))}\n'
|
|
f'exec {shlex.quote(str(SRCDIR / "support" / "lsh.sh"))} "$@"\n'
|
|
)
|
|
wrapper.chmod(0o755)
|
|
|
|
rmtree(dst)
|
|
dst.mkdir()
|
|
os.environ['RSYNC_RSH'] = rsh_cmd(str(wrapper))
|
|
run_rsync('-r', '--max-alloc=0', f'--rsync-path={rsync_path_arg()}',
|
|
f'localhost:{src}/', f'{dst}/')
|
|
del os.environ['RSYNC_RSH']
|
|
|
|
if (dst / 'file.txt').read_text() != 'hello\n':
|
|
test_fail('--max-alloc=0 did not copy the file over the remote shell')
|
|
|
|
logged = argv_log.read_text() if argv_log.exists() else ''
|
|
if not logged:
|
|
test_fail('the remote-shell wrapper logged no command line')
|
|
if '--max-alloc=0' not in f' {logged} '.replace('\n', ' '):
|
|
test_fail('--max-alloc=0 was not forwarded verbatim; the peer was sent:\n'
|
|
f'{logged}'
|
|
'\nA resolved number here would be rejected as "too large" by a '
|
|
'peer with a smaller SIZE_MAX.')
|
|
|
|
# --- 3. the upper bound still holds -----------------------------------------
|
|
|
|
# 8192P is one step past SIZE_ARG_MAX (SIZE_MAX/2) on a 64-bit build; on a
|
|
# 32-bit one the P multiplier alone already exceeds it. Either way: too large.
|
|
expect_fail(rsync_argv('--max-alloc=8192P', f'{src}/', f'{dst}/'), 'is too large')
|
|
|
|
# And the min-value message must keep advertising a spelling that works.
|
|
expect_fail(rsync_argv('--max-alloc=1', f'{src}/', f'{dst}/'),
|
|
'or 0 for unlimited')
|
|
|
|
print('max-alloc-zero: 0 is accepted, forwarded verbatim, and the bound holds')
|