Bug 6151 - --safe-links can be fooled by adding extra slashes to the path
Summary: --safe-links can be fooled by adding extra slashes to the path
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.1.0
Hardware: x64 Linux
: P3 normal (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-02 15:54 UTC by Erik Sjölund
Modified: 2009-03-03 11:06 UTC (History)
0 users

See Also:


Attachments
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 
 --copy-unsafe-links 
 --safe-links 
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:
http://samba.anu.edu.au/ftp/rsync/nightly/rsync-HEAD-20090228-1730GMT.tar.gz
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.