Bug 6151 - --safe-links can be fooled by adding extra slashes to the path
--safe-links can be fooled by adding extra slashes to the path
Product: rsync
Classification: Unclassified
Component: core
x64 Linux
: P3 normal
: ---
Assigned To: Wayne Davison
Rsync QA Contact
Depends on:
  Show dependency treegraph
Reported: 2009-03-02 15:54 UTC by Erik Sjölund
Modified: 2009-03-03 11:06 UTC (History)
0 users

See Also:

fixes this bug (672 bytes, patch)
2009-03-02 16:44 UTC, Erik Sjölund
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erik Sjölund 2009-03-02 15:54:00 UTC
The rsync option flags 
are not working correctly. It is possible to fool the function 
unsafe_symlink(const char *dest, const char *src) in util.c
by adding extra slashes as the following example shows:

$ mkdir /tmp/a
$ cd /tmp/a
$ ln -s ../../etc/passwd passwd1
$ ln -s .////../../etc/passwd passwd2
$ rsync -av --safe-links /tmp/a/ /tmp/b
sending incremental file list
created directory /tmp/b
ignoring unsafe symlink "/tmp/b/passwd1" -> "../../etc/passwd"
passwd2 -> .////../../etc/passwd

sent 115 bytes  received 18 bytes  266.00 bytes/sec
total size is 37  speedup is 0.28
$ ls -l /tmp/b
total 0
lrwxrwxrwx 1 esjolund users 21 2009-03-02 22:33 passwd2 -> .////../../etc/passwd

We see that rsync correctly detects /tmp/a/passwd1 as being a symlink pointing outside the tree and rsync therefore ignores that symlink. But rsync fails to discover that /tmp/a/passwd2 also points outside the tree.
The rsync version used in the above example was compiled from:
on a Centos Linux 5.2 x86_64
Comment 1 Erik Sjölund 2009-03-02 16:44:30 UTC
Created attachment 3965 [details]
fixes this bug

This patch fixes this bug.

I am still a bit worried about the foor loop directly after  

/* find out what our safety margin is */

It tries to analyze "src" but, I don't know exactly what it is trying to do. For instance why does it have 
depth = 0 
and not --depth? How does take care of symbolic links inside the "src" path? In general ".." in the root directory is also the root directory. Is that considered? I know too little about what restrictions the arguments coming into this function have already gone through ( filtering, cleaning up ).
Comment 2 Wayne Davison 2009-03-03 11:06:28 UTC
Thanks.  I've committed an improved version of the unsafe_symlinks() function and put some extra items into the test case.

As for why the depth scanner is setting depth = 0 for any ".." in the src:  it's being extra conservative.  ".." should never appear in the source, but if it does, we just restart depth counting at that point.