The Samba-Bugzilla – Bug 6151
--safe-links can be fooled by adding extra slashes to the path
Last modified: 2009-03-03 11:06:28 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
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
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 ).
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.