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

diff --git a/testsuite/safe-links.test b/testsuite/safe-links.test
new file mode 100644 (file)
index 0000000..6e95a4b
--- /dev/null
+++ b/testsuite/safe-links.test
@@ -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

diff --git a/testsuite/unsafe-byname.test b/testsuite/unsafe-byname.test
index 75e720145a7aadf0db7f09d5010dbfd8b70f20a5..d2e318ef4fb472dbc0d7d948aedaf6da45673dcd 100644 (file)
--- a/testsuite/unsafe-byname.test
+++ b/testsuite/unsafe-byname.test
@@ -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
 

diff --git a/util1.c b/util1.c
index da50ff1e8e457e3722de7d50beb1fd26b409ba31..f260d39801904d4aea9c4bb74efed9f525bf0956 100644 (file)
--- a/util1.c
+++ b/util1.c
@@ -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;
@@ -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. */