mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-09-17 23:57:46 -04:00
Andrew Tridgell
05bd16a469
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.
33 lines
1.1 KiB
Python
33 lines
1.1 KiB
Python
#!/usr/bin/env python3
|
|
import os
|
|
import subprocess
|
|
|
|
from rsyncfns import SCRATCHDIR, rmtree, rsync_argv, start_test_daemon, test_fail, write_daemon_conf
|
|
|
|
base = SCRATCHDIR / 'exec-env-escape'
|
|
rmtree(base)
|
|
src = base / 'src'
|
|
src.mkdir(parents=True)
|
|
(src / 'f').write_text('payload\n')
|
|
sentinel = base / 'pwned'
|
|
|
|
os.environ['RSYNC_REQUEST'] = f"ok;touch {sentinel};#"
|
|
conf = write_daemon_conf([
|
|
('execmod', {'path': str(src), 'read only': 'yes',
|
|
'pre-xfer exec': 'sh -c "printf %RSYNC_REQUEST% >/dev/null"'}),
|
|
])
|
|
url = start_test_daemon(conf, 12936)
|
|
proc = subprocess.run(
|
|
rsync_argv('-r', f'{url}execmod/'),
|
|
stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
|
|
os.environ.pop('RSYNC_REQUEST', None)
|
|
|
|
if sentinel.exists():
|
|
test_fail("daemon exec-hook %RSYNC_REQUEST% expansion executed injected shell syntax")
|
|
# A value carrying shell syntax is refused outright rather than quoted, so the
|
|
# hook must not run and the transfer must fail closed.
|
|
if proc.returncode == 0:
|
|
test_fail("daemon exec-hook accepted a %RSYNC_REQUEST% holding shell syntax")
|
|
|
|
print("daemon-exec-rsync-env-shell-escape: %RSYNC_*% holding shell syntax is refused")
|