Skip to content
Prev Previous commit
Next Next commit
make --safe-links stricter
when --safe-links is used also reject links where a '../' component is
included in the destination as other than the leading part of the
filename
  • Loading branch information
tridge committed Jan 14, 2025
commit e2715ef6ca4e03405f95eb4551363a199778f53f
55 changes: 55 additions & 0 deletions testsuite/safe-links.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#!/bin/sh

. "$suitedir/rsync.fns"

test_symlink() {
is_a_link "$1" || test_fail "File $1 is not a symlink"
}

test_regular() {
if [ ! -f "$1" ]; then
test_fail "File $1 is not regular file or not exists"
fi
}

test_notexist() {
if [ -e "$1" ]; then
test_fail "File $1 exists"
fi
if [ -h "$1" ]; then
test_fail "File $1 exists as a symlink"
fi
}

cd "$tmpdir"

mkdir from

mkdir "from/safe"
mkdir "from/unsafe"

mkdir "from/safe/files"
mkdir "from/safe/links"

touch "from/safe/files/file1"
touch "from/safe/files/file2"
touch "from/unsafe/unsafefile"

ln -s ../files/file1 "from/safe/links/"
ln -s ../files/file2 "from/safe/links/"
ln -s ../../unsafe/unsafefile "from/safe/links/"
ln -s a/a/a/../../../unsafe2 "from/safe/links/"

#echo "LISTING FROM"
#ls -lR from

echo "rsync with relative path and just -a"
$RSYNC -avv --safe-links from/safe/ to

#echo "LISTING TO"
#ls -lR to

test_symlink to/links/file1
test_symlink to/links/file2
test_notexist to/links/unsafefile
test_notexist to/links/unsafe2
2 changes: 1 addition & 1 deletion testsuite/unsafe-byname.test
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test_unsafe ..//../dest from/dir unsafe
test_unsafe .. from/file safe
test_unsafe ../.. from/file unsafe
test_unsafe ..//.. from//file unsafe
test_unsafe dir/.. from safe
test_unsafe dir/.. from unsafe
test_unsafe dir/../.. from unsafe
test_unsafe dir/..//.. from unsafe

Expand Down
26 changes: 25 additions & 1 deletion util1.c
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,14 @@ int handle_partial_dir(const char *fname, int create)
*
* "src" is the top source directory currently applicable at the level
* of the referenced symlink. This is usually the symlink's full path
* (including its name), as referenced from the root of the transfer. */
* (including its name), as referenced from the root of the transfer.
*
* NOTE: this also rejects dest names with a .. component in other
* than the first component of the name ie. it rejects names such as
* a/b/../x/y. This needs to be done as the leading subpaths 'a' or
* 'b' could later be replaced with symlinks such as a link to '.'
* resulting in the link being transferred now becoming unsafe
*/
int unsafe_symlink(const char *dest, const char *src)
{
const char *name, *slash;
Expand All @@ -1328,6 +1335,23 @@ int unsafe_symlink(const char *dest, const char *src)
if (!dest || !*dest || *dest == '/')
return 1;

// reject destinations with /../ in the name other than at the start of the name
const char *dest2 = dest;
while (strncmp(dest2, "../", 3) == 0) {
dest2 += 3;
while (*dest2 == '/') {
// allow for ..//..///../foo
dest2++;
}
}
if (strstr(dest2, "/../"))
return 1;

// reject if the destination ends in /..
const size_t dlen = strlen(dest);
if (dlen > 3 && strcmp(&dest[dlen-3], "/..") == 0)
return 1;

/* find out what our safety margin is */
for (name = src; (slash = strchr(name, '/')) != 0; name = slash+1) {
/* ".." segment starts the count over. "." segment is ignored. */
Expand Down