Package: rsync
Version: 3.2.7-1
Severity: normal
X-Debbugs-Cc: jsmith6...@gmx.net

Dear Maintainer,

I found a regression in rrsync, which came with the change from Perl (Bullseye)
to Python (Bookworm), and would like to propose a bug fix.


   * What led up to the situation?

I am using backupninja to create local backups from a remote server. With
Bullseye, I had to use the following line in the configuration file to make it
work with rrsync:
    include = .

backupninja converts this to the command
    rsync <arguments> user@remoteserver:/./ /localbackupdir/
to read the complete subdirectory defined by rrsync (see below) and store it in
the local backup directory.


authorized_keys on the remote server:
    command="/usr/bin/rrsync -ro /path/to/restricted/source/",restrict ssh-rsa
...


Starting with Bookworm, rrsync breaks the backup process with the error "unsafe
arg":
(Output of die('unsafe arg:', orig_arg, [arg, real_arg]) )
    unsafe arg: /./, [/path/to/restricted/source/., /path/to/restricted/source]

Note that arg is not equal to real_arg, since the trailing "/./" of the
original argument (orig_arg) is not completely stripped from arg.


   * What exactly did you do (or not do) that was effective (or ineffective)?

I found that in rrsync, there is a directory argument check that is too
restrictive to work with above (valid) string:

rrsync 3.2.7 (as in Bookworm):
https://salsa.debian.org/debian/rsync/-/blob/debian/3.2.7-1/support/rrsync?ref_type=tags#L309

    ret = [ ]
    for arg in got:
        if args.dir != '/' and arg != '.' and (typ == 3 or (typ == 2 and not
am_sender)):
            arg_has_trailing_slash = arg.endswith('/')
            if arg_has_trailing_slash:
                arg = arg[:-1]
            else:
                arg_has_trailing_slash_dot = arg.endswith('/.')
                if arg_has_trailing_slash_dot:
                    arg = arg[:-2]
            real_arg = os.path.realpath(arg)
            if arg != real_arg and not real_arg.startswith(args.dir_slash):
                die('unsafe arg:', orig_arg, [arg, real_arg])
            if arg_has_trailing_slash:
                arg += '/'
            elif arg_has_trailing_slash_dot:
                arg += '/.'
            if opt == 'arg' and arg.startswith(args.dir_slash):
                arg = arg[args.dir_slash_len:]
                if arg == '':
                    arg = '.'
        ret.append(arg)

After a small patch, the code seems still valid for me, but allowing a trailing
"/" after a trailing "/.", therefore allowing a trailing "/./".

My proposed patch:

    ret = [ ]
    for arg in got:
        if args.dir != '/' and arg != '.' and (typ == 3 or (typ == 2 and not
am_sender)):
1.          arg_has_trailing_slash = arg.endswith('/')
            if arg_has_trailing_slash:
                arg = arg[:-1]
2.          arg_has_trailing_slash_dot = arg.endswith('/.')
            if arg_has_trailing_slash_dot:
                arg = arg[:-2]
3.          real_arg = os.path.realpath(arg)
            if arg != real_arg and not real_arg.startswith(args.dir_slash):
                die('unsafe arg:', orig_arg, [arg, real_arg])
4.          if arg_has_trailing_slash_dot:
                arg += '/.'
5.          if arg_has_trailing_slash:
                arg += '/'
            if opt == 'arg' and arg.startswith(args.dir_slash):
                arg = arg[args.dir_slash_len:]
                if arg == '':
                    arg = '.'
        ret.append(arg)

1. Trailing "/" is removed.
2. Trailing "/." is removed.
3. Directory is checked.
4. Trailing "/." is appended again.
5. Trailing "/" is appended again.

Please check carefully, I am not familiar with the Python language.


   * What was the outcome of this action?

The check in line 320 (
https://salsa.debian.org/debian/rsync/-/blob/debian/3.2.7-1/support/rrsync?ref_type=tags#L320
) does not fail, which is the intended result.


Thank you in advance.


-- System Information:
Debian Release: 12.1
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 6.1.0-10-amd64 (SMP w/8 CPU threads; PREEMPT)
Kernel taint flags: TAINT_WARN, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages rsync depends on:
ii  init-system-helpers        1.65.2
ii  libacl1                    2.3.1-3
ii  libc6                      2.36-9+deb12u1
ii  liblz4-1                   1.9.4-1
ii  libpopt0                   1.19+dfsg-1
ii  libssl3                    3.0.9-1
ii  libxxhash0                 0.8.1-1
ii  libzstd1                   1.5.4+dfsg2-5
ii  lsb-base                   11.6
ii  sysvinit-utils [lsb-base]  3.06-4
ii  zlib1g                     1:1.2.13.dfsg-1

rsync recommends no packages.

Versions of packages rsync suggests:
ii  openssh-client       1:9.2p1-2
ii  openssh-server       1:9.2p1-2
ii  python3              3.11.2-1+b1
pn  python3-braceexpand  <none>

-- no debconf information

Reply via email to