From 535f8f816539ba681ef0f12015d2cb587ae61b6d Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <andrew@tridgell.net>
Date: Sat, 23 Nov 2024 15:15:53 +1100
Subject: [PATCH] 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
---
 testsuite/safe-links.test    | 55 ++++++++++++++++++++++++++++++++++++
 testsuite/unsafe-byname.test |  2 +-
 util1.c                      | 26 ++++++++++++++++-
 3 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100644 testsuite/safe-links.test

diff --git a/testsuite/safe-links.test b/testsuite/safe-links.test
new file mode 100644
index 00000000..6e95a4b9
--- /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 75e72014..d2e318ef 100644
--- 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 da50ff1e..f260d398 100644
--- 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. */
-- 
2.34.1