diff --git a/support/rrsync b/support/rrsync index e11457009..060c8f086 100755 --- a/support/rrsync +++ b/support/rrsync @@ -694,8 +694,13 @@ def validated_arg(opt, arg, typ=3, wild=False): if arg.startswith('./'): arg = arg[1:] arg = arg.replace('//', '/') + # One leading slash means the root of the restricted tree. + arg = re.sub(r'^/+', '/', arg) is_absolute_arg = args.absolute and opt == 'arg' and args.dir != '/' and (arg == args.dir or arg.startswith(args.dir_slash)) - if not is_absolute_arg: + is_root_relative_arg = arg.startswith('/') and not is_absolute_arg + if is_root_relative_arg and args.dir != '/': + arg = args.dir + arg + elif not is_absolute_arg: arg = arg.lstrip('/') if args.dir != '/': if HAS_DOT_DOT_RE.search(arg): diff --git a/testsuite/rrsync-root-relative-paths_test.py b/testsuite/rrsync-root-relative-paths_test.py new file mode 100644 index 000000000..56ddbf8c0 --- /dev/null +++ b/testsuite/rrsync-root-relative-paths_test.py @@ -0,0 +1,128 @@ +#!/usr/bin/env python3 +"""Leading slashes through rrsync must remain relative to the restricted root.""" + +import os +import shlex +import subprocess + +from rsyncfns import ( + RSYNC, SCRATCHDIR, makepath, patched_rrsync, rmtree, rsync_argv, + rsync_path_arg, test_fail, +) + +T = 1234567890 + +base = SCRATCHDIR / 'rrsync-root-relative-paths' +rmtree(base) +restricted = base / 'restricted' +source = base / 'source' +outside = base / 'outside' +pulled = base / 'pulled' +makepath(restricted / 'previous', source, outside, pulled) + +(restricted / 'previous' / 'file').write_text('unchanged\n') +(source / 'file').write_text('unchanged\n') +(outside / 'secret').write_text('outside\n') +os.utime(restricted / 'previous' / 'file', (T, T)) +os.utime(source / 'file', (T, T)) + +shim = base / 'rsync-shim' +shim.write_text('#!/bin/sh\nexec ' + rsync_path_arg(RSYNC) + ' "$@"\n') +shim.chmod(0o755) +rrsync = patched_rrsync(base, rsync_path=str(shim)) + +rsh = base / 'fake-rsh' +rsh.write_text( + '#!/bin/sh\n' + 'shift\n' + 'SSH_ORIGINAL_COMMAND="$*"\n' + 'export SSH_ORIGINAL_COMMAND\n' + 'exec %s %s\n' % (shlex.quote(str(rrsync)), shlex.quote(str(restricted)))) +rsh.chmod(0o755) + +rsh_absolute = base / 'fake-rsh-absolute' +rsh_absolute.write_text( + '#!/bin/sh\n' + 'shift\n' + 'SSH_ORIGINAL_COMMAND="$*"\n' + 'export SSH_ORIGINAL_COMMAND\n' + 'exec %s -absolute %s\n' + % (shlex.quote(str(rrsync)), shlex.quote(str(restricted)))) +rsh_absolute.chmod(0o755) + + +def run(*args): + return subprocess.run( + rsync_argv('-e', str(rsh), *args), + capture_output=True, text=True, timeout=30, + ) + + +# A leading slash denotes the root of the restricted tree, not an empty path. +listed = run('--list-only', 'dummy:/') +listed_ctx = (f'rc={listed.returncode}, stdout={listed.stdout.strip()!r}, ' + f'stderr={listed.stderr.strip()!r}') +if listed.returncode != 0 or 'previous' not in listed.stdout: + test_fail(f'listing the restricted root with / failed ({listed_ctx})') + +# Option operands use the same convention. The basis is a sibling of the +# destination, so stripping the slash and leaving a relative name makes rsync +# look for destination/previous and silently transfer a full copy. +linked = run('-a', '--link-dest=/previous', str(source) + '/', 'dummy:/current') +linked_ctx = (f'rc={linked.returncode}, stdout={linked.stdout.strip()!r}, ' + f'stderr={linked.stderr.strip()!r}') +current = restricted / 'current' / 'file' +previous = restricted / 'previous' / 'file' +if linked.returncode != 0 or not current.is_file(): + test_fail(f'root-relative --link-dest transfer failed ({linked_ctx})') +current_stat = os.stat(current) +previous_stat = os.stat(previous) +if ((current_stat.st_dev, current_stat.st_ino) + != (previous_stat.st_dev, previous_stat.st_ino)): + test_fail(f'root-relative --link-dest did not hard-link to its basis ' + f'({linked_ctx})') + +# Repeated leading slashes have the same restricted-root meaning. Assert the +# authorised path still works rather than satisfying confinement by rejecting +# every repeated-slash path. +repeated = run('-a', 'dummy:///previous/file', str(pulled) + '/') +if (repeated.returncode != 0 + or not (pulled / 'file').is_file() + or (pulled / 'file').read_text() != 'unchanged\n'): + test_fail('repeated leading slashes did not resolve beneath the ' + f'restricted root (rc={repeated.returncode}, ' + f'stderr={repeated.stderr.strip()!r})') + +# The explicit -absolute mode continues to accept a complete server path under +# the restricted directory; root-relative handling must not reinterpret it. +rmtree(pulled) +pulled.mkdir() +absolute = subprocess.run( + rsync_argv('-a', '-e', str(rsh_absolute), + 'dummy:' + str(restricted / 'previous' / 'file'), + str(pulled) + '/'), + capture_output=True, text=True, timeout=30, +) +if (absolute.returncode != 0 + or not (pulled / 'file').is_file() + or (pulled / 'file').read_text() != 'unchanged\n'): + test_fail('-absolute no longer accepts an in-tree server path ' + f'(rc={absolute.returncode}, stderr={absolute.stderr.strip()!r})') + +# Repeated leading slashes must not regain host-root meaning in either transfer +# direction. The requested path is the absolute spelling of a file outside +# the restricted tree; a vulnerable wrapper would read or replace that file. +outside_arg = '///' + str(outside / 'secret').lstrip('/') +rmtree(pulled) +pulled.mkdir() +escaped_pull = run('-a', 'dummy:' + outside_arg, str(pulled) + '/') +if (pulled / 'secret').exists(): + test_fail('repeated leading slashes escaped the restricted root on pull') + +replacement = base / 'replacement' +replacement.write_text('replacement\n') +escaped_push = run('-a', str(replacement), 'dummy:' + outside_arg) +if (outside / 'secret').read_text() != 'outside\n': + test_fail('repeated leading slashes escaped the restricted root on push') + +print('rrsync preserves restricted-root paths without repeated-slash escapes')