From 290ffcb488bea5caec6d76a34ea8368d00c68875 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Wed, 28 Jan 2015 17:11:12 +0000 Subject: Allow arbitrary symlink targets again * src/util.c (symlink_target_is_valid): Remove. (move_file): Remove symlink target checking. * tests/symlinks: Update test case. --- src/util.c | 28 ---------------------------- tests/symlinks | 57 +++++++++++++++++++++++++++++++++++++++------------------ 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 < symlink-target.diff < follow-symlink.diff < bad-symlink-target1.diff < bad-symlink-target2.diff <