rsync - Mar 2019 - [Bug 13827] New: despite --copy-unsafe-links, rsync does not copy the referent of symlinks that point one level outside the copied tree

https://bugzilla.samba.org/show_bug.cgi?id=13827

            Bug ID: 13827
           Summary: despite --copy-unsafe-links, rsync does not copy the
                    referent of symlinks that point one level outside the
                    copied tree
           Product: rsync
           Version: 3.1.3
          Hardware: All
                OS: All
            Status: NEW
          Severity: major
          Priority: P5
         Component: core
          Assignee: wayne at opencoder.net
          Reporter: jan at bredereke-net.de
        QA Contact: rsync-qa at samba.org

Created attachment 14913
  --> https://bugzilla.samba.org/attachment.cgi?id=14913&action=edit
short shell script demonstrating the bug

Despite --copy-unsafe-links, rsync does not copy the referent of symlinks that
point one level outside the copied tree. The short shell script attached
demonstrates the problem. It also demonstrates two other cases where the checks
work as intended. The problem appears to be an off-by-one error in a check.

Accessing things outside the copied tree through a symlink is probably a
security problem. However, the restriction to only one directory level too far
makes it more difficult to expoit.

1)
Tested on Lubuntu 18.04.1 LTS

2)
rsync version 3.1.2, protocol version 31
(The most current version of rsync is 3.1.3. But its release notes do not
mention this bug to be fixed.)
The change & release notes of Lubuntu 18.04.1 do not mention rsync.
The bug tracker Ubuntu Launchpad does not mention this bug.

3)
I expected any symlink pointing outside the copied tree to be converted into a
copy, when I use --copy-unsafe-links.

4)
A symlink pointing just one level outside the copied tree is not converted.
This is always reproducible, see the demo script attached.

I submitted this bug to Ubuntu Launchpad first. But they told me to submit it
here. (Since I declared it a security relevant bug, it became non-public by
default.)
https://bugs.launchpad.net/ubuntu/+source/rsync/+bug/1816586

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

https://bugzilla.samba.org/show_bug.cgi?id=13827

Wayne Davison <wayne at opencoder.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|NEW                         |RESOLVED

--- Comment #1 from Wayne Davison <wayne at opencoder.net> ---
In the command:

rsync --archive --copy-unsafe-symlinks home ../rootdir2

The top dir in the tree is "." with "home" being something
that you want to
transfer in that tree.  Thus, anything in that tree is fair game in rsync's
algorithm.  You should instead run this (which was your second test):

rsync --archive --copy-unsafe-symlinks home/ ../rootdir2/home/

Which makes sure that nothing exits the home dir's tree.

The reason for this is that someone might have specified more dirs to copy in
the past (such as "home bin") and then do a partial copy within that
tree with
just "home" and they don't want the symlinks mangled in that case.
Rsync makes
the rule that the cut-off point is the top of the tree for the transfer, not an
item within the tree.  You can always tell what the top of the tree is by
seeing what part of the path rsync isn't mentioning.  If the name is
mentioned
(as "home" is in the first case) it isn't the top of the transfer.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.

https://bugzilla.samba.org/show_bug.cgi?id=13827

--- Comment #2 from Jan Bredereke <jan at bredereke-net.de> ---
(In reply to Wayne Davison from comment #1)

Thanks, I see your point now.

Thus the problem is that the subject is rather complex, and the
man-page is quite terse on these details. A user can make a mistake
easily, as my case proves. Because of the security implications my
little script demonstrates, this is undesirable.

Therefore, I propose to improve the man-page.

The original text of the option in question, and of the next,
related option is:

  --copy-unsafe-links
         This  tells  rsync  to  copy the referent of symbolic links that
         point outside the  copied  tree.   Absolute  symlinks  are  also
         treated  like  ordinary  files,  and  so are any symlinks in the
         source path itself when --relative is used.  This option has  no
         additional effect if --copy-links was also specified.

  --safe-links
         This  tells  rsync to ignore any symbolic links which point out‐
         side the copied tree. All absolute symlinks  are  also  ignored.
         Using  this option in conjunction with --relative may give unex‐
         pected results.

For both options, I would add after the first sentence: "See the
SYMBOLIC LINKS section." -- When I looked up
"--copy-unsafe-links",
I missed to notice the existence of this helpful section.

Furthermore, the notion of "copied tree" is not explained in the
man-page. Even more, the SYMBOLIC LINKS section is a bit inconsistent
with this respect. The relevant current text is:

  Symbolic  links  are  considered  unsafe  if they are absolute symlinks
  (start with /), empty, or if they contain  enough  ".."  components 
to
  ascend from the directory being copied.

It should not be "directory being copied", but "tree being
copied".

When this is fixed, I propose to continue the paragraph with the
following definition of the copied tree: "If the source given is
bar/baz, then the top of the tree copied is below bar. If the source
given is bar/, which contains baz, then the top of the tree copied
is below bar, too. A symbolic link slink1 in directory bar, pointing
to baz, is considered safe. A symbolic link slink2 in the same
place, pointing to ../baz, is considered unsafe."

If you consider this too lengthy, the last two sentences with the
examples could be left out.

I now consider the behaviour I discovered as being consistent. But
there is another aspect which appears to me as inconsistent with
respect to the rules you just explained. The man-page writes (in a
nice, clear way):

  A  trailing slash on the source changes this behavior to avoid creating
  an additional directory level at the destination.  You can think  of  a
  trailing / on a source as meaning "copy the contents of this
directory"
  as opposed to "copy the directory by  name",  but  in  both  cases 
the
  attributes  of the containing directory are transferred to the contain‐
  ing directory on the destination.  In other words, each of the  follow‐
  ing  commands copies the files in the same way, including their setting
  of the attributes of /dest/foo:

         rsync -av /src/foo /dest
         rsync -av /src/foo/ /dest/foo

In the second example, foo is above the top of the tree, but its
attributes are set, nevertheless. This is necessary if you need to
transfer /. But the concept of the tree copied becomes even harder
to grasp because of this extra rule. Unfortunately, I have no idea
on how to improve the situation here.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.