Allow arbitrary symlink targets again

* src/util.c (symlink_target_is_valid): Remove.
(move_file): Remove symlink target checking.
* tests/symlinks: Update test case.

2 files changed, 39 insertions, 46 deletions

diff --git a/src/util.c b/src/util.c
index eba3943..fae628a 100644
--- a/src/util.c
+++ b/src/util.c
@@ -428,23 +428,6 @@ create_backup (char const *to, const struct stat *to_st, bool leave_original)
     }
 }
 
-/* Only allow symlink targets which are relative and free of ".." components:
- * otherwise, the operating system may follow one of those symlinks in a
- * pathname component, leading to a path traversal vulnerability.
- *
- * An alternative to disallowing many kinds of symlinks would be to implement
- * path traversal in user space using openat() without following symlinks
- * altogether.
- */
-static bool
-symlink_target_is_valid (char const *target, char const *to)
-{
-  bool is_valid = filename_is_safe (target);
-
-  /* Allow any symlink target if we are in the filesystem root.  */
-  return is_valid || cwd_is_root (to);
-}
-
 /* Move a file FROM (where *FROM_NEEDS_REMOVAL is nonzero if FROM
    needs removal when cleaning up at the end of execution, and where
    *FROMST is FROM's status if known),
@@ -488,17 +471,6 @@ move_file (char const *from, bool *from_needs_removal,
 	    read_fatal ();
 	  buffer[size] = 0;
 
-	  /* If we are allowed to create a file with an absolute path name,
-	     anywhere, we also don't need to worry about symlinks that can
-	     leave the working directory.  */
-	  if (! (IS_ABSOLUTE_FILE_NAME (to)
-		 || symlink_target_is_valid (buffer, to)))
-	    {
-	      fprintf (stderr, "symbolic link target '%s' is invalid\n",
-		       buffer);
-	      fatal_exit (0);
-	    }
-
 	  if (! backup)
 	    {
 	      if (safe_unlink (to) == 0)
diff --git a/tests/symlinks b/tests/symlinks
index 04a9b73..2e85da7 100644
--- a/tests/symlinks
+++ b/tests/symlinks
@@ -152,20 +152,45 @@ ncheck 'test ! -L symlink'
 # guarantee that they won't end up higher up in the working tree than we think;
 # the path to the symlink may follow symlinks itself.
 #
-#cat > symlink-target.diff <<EOF
-#diff --git a/dir/foo b/dir/foo
-#new file mode 120000
-#index 0000000..cad2309
-#--- /dev/null
-#+++ b/dir/foo
-#@@ -0,0 +1 @@
-#+../foo
-#\ No newline at end of file
-#EOF
-#
-#check 'patch -p1 < symlink-target.diff || echo "Status: $?"' <<EOF
-#patching symbolic link dir/foo
-#EOF
+cat > symlink-target.diff <<EOF
+diff --git a/dir/foo b/dir/foo
+new file mode 120000
+index 0000000..cad2309
+--- /dev/null
++++ b/dir/foo
+@@ -0,0 +1 @@
++../foo
+\ No newline at end of file
+EOF
+
+check 'patch -p1 < symlink-target.diff || echo "Status: $?"' <<EOF
+patching symbolic link dir/foo
+EOF
+
+rm -f dir/foo
+cat > follow-symlink.diff <<EOF
+diff --git a/dir/foo b/dir/foo
+new file mode 120000
+index 0000000..cad2309
+--- /dev/null
++++ b/dir/foo
+@@ -0,0 +1 @@
++..
+\ No newline at end of file
+diff --git a/dir/foo/bar b/dir/foo/bar
+new file mode 100644
+index 0000000..2ab772d
+--- /dev/null
++++ b/dir/foo/bar
+@@ -0,0 +1 @@
++created in ..
+EOF
+
+check 'patch -f -p1 < follow-symlink.diff || echo "Status: $?"' <<EOF
+patching symbolic link dir/foo
+Refusing to follow symbolic link dir/foo
+Status: 2
+EOF
 
 cat > bad-symlink-target1.diff <<EOF
 diff --git a/bar b/bar
@@ -180,8 +205,6 @@ EOF
 
 check 'patch -p1 < bad-symlink-target1.diff || echo "Status: $?"' <<EOF
 patching symbolic link bar
-symbolic link target '/bar' is invalid
-Status: 2
 EOF
 
 cat > bad-symlink-target2.diff <<EOF
@@ -197,8 +220,6 @@ EOF
 
 check 'patch -p1 < bad-symlink-target2.diff || echo "Status: $?"' <<EOF
 patching symbolic link baz
-symbolic link target '../baz' is invalid
-Status: 2
 EOF
 
 # --------------------------------------------------------------