Allow arbitrary symlink targets again

* src/util.c (symlink_target_is_valid): Remove.
(move_file): Remove symlink target checking.
* tests/symlinks: Update test case.
This commit is contained in:
Tim Waugh 2015-01-28 17:11:12 +00:00 committed by Andreas Gruenbacher
parent b72e3be5c8
commit 290ffcb488
2 changed files with 39 additions and 46 deletions

View File

@ -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)

View File

@ -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
# --------------------------------------------------------------