For years cp and friends have been subject to a symlink attack, in that
seemingly-ordinary commands like 'cp a b' can overwrite arbitrary directories
that the user has access to, if b's parent directory is world-writable and is
not sticky and is manipulated by a malicious user. To help ameliorate this
problem, I've installed the attached patch into coreutils. Although it does not
detect every instance of this problem, the goal is that its heuristic should
avoid false positives while catching and reporting and refusing to act on most
instances.
As this is not an upward-compatible change, I installed this patch right after a
coreutils release, in the hope that we will have time to shake out any problems
with it (including reverting it entirely :-) before the next release.
From 44ccd1c4657703b15971b0670b9716a25244a358 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 18 Sep 2017 18:54:52 -0700
Subject: [PATCH] copy: check for vulnerable target dirs
* NEWS, doc/coreutils.texi (Target directory): Document this.
* src/cp.c, src/install.c, src/ln.c, src/mv.c: Include targetdir.h.
(target_directory_operand): Use the new targetdir_operand_type
function to check for vulnerable target directories.
* src/cp.c (stat_target_operand): New function.
(target_directory_operand, do_copy): Use it.
* src/local.mk (noinst_HEADERS): Add src/targetdir.h.
(src_ginstall_SOURCES, src_cp_SOURCES, src_ln_SOURCES)
(src_mv_SOURCES): Add src/targetdir.c.
* src/targetdir.c, src/targetdir.h: New files.
* tests/mv/vulnerable-target.sh: New test.
* tests/local.mk (all_root_tests): Add it.
---
NEWS | 11 +++++
doc/coreutils.texi | 33 +++++++++++++-
src/cp.c | 37 +++++++++++-----
src/install.c | 32 +++++++++-----
src/ln.c | 34 +++++++++-----
src/local.mk | 13 +++---
src/mv.c | 22 +++++++---
src/targetdir.c | 100 ++++++++++++++++++++++++++++++++++++++++++
src/targetdir.h | 4 ++
tests/local.mk | 1 +
tests/mv/vulnerable-target.sh | 38 ++++++++++++++++
11 files changed, 280 insertions(+), 45 deletions(-)
create mode 100644 src/targetdir.c
create mode 100644 src/targetdir.h
create mode 100755 tests/mv/vulnerable-target.sh
diff --git a/NEWS b/NEWS
index ca36063..9a005c5 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,17 @@ GNU coreutils NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** Changes in behavior
+
+ cp, install, ln, and mv by default now reject some usages that are
+ likely vulnerable to hijacking, to foil some attacks on unsafe
+ shared target directories. For example, if /tmp/risky is
+ world-writable and /tmp/risky/d is a directory, 'cp f /tmp/risky/d'
+ now fails because the target directory is vulnerable. To suppress
+ the vulnerability heuristic, append '/' to the name of the target
+ directory, or use the -t or -T options, or (for cp, ln, and mv) set
+ the POSIXLY_CORRECT environment variable.
+
** Bug fixes
ptx -S no longer infloops for a pattern which returns zero-length matches.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index eb17462..0c6e2c5 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -1282,7 +1282,38 @@ The @command{cp}, @command{install}, @command{ln}, and @command{mv}
commands normally treat the last operand specially when it is a
directory or a symbolic link to a directory. For example, @samp{cp
source dest} is equivalent to @samp{cp source dest/source} if
-@file{dest} is a directory. Sometimes this behavior is not exactly
+@file{dest} is a directory.
+
+@cindex vulnerable target directory
+@vindex POSIXLY_CORRECT
+However, if the destination is a directory whose name could be that of
+an ordinary file, and the directory's parent appears to be vulnerable
+to an attack by another user, then these programs report the
+vulnerability and fail. If you are not worried about such an attack
+you can suppress this heuristic by appending @samp{/} to the
+destination's name, or by using the @option{--target-directory}
+(@option{-t}) or @option{--no-target-directory} (@option{-T}) options.
+(For @command{cp}, @command{ln}, and @command{mv} you can also
+suppress the heuristic by setting the @env{POSIXLY_CORRECT}
+environment variable.) For example, if @file{/tmp/risky/d} is a
+directory whose parent @file{/tmp/risky} is is world-writable and is
+not sticky, the command @samp{cp passwd /tmp/risky/d} fails with
+a diagnostic reporting a vulnerable target directory, as an attacker
+could replace @file{/tmp/risky/d} by a symbolic link to a victim
+directory while @command{cp} is running. In this example, you can
+suppress the heuristic by issuing one of the following shell commands
+instead:
+
+@example
+# These can be hijacked if /tmp/risky is rwxrwxrwx!
+cp passwd /tmp/risky/d/
+cp -t /tmp/risky/d passwd
+cp -T passwd /tmp/risky/d/passwd
+POSIXLY_CORRECT=yes cp passwd /tmp/risky/d
+@end example
+
+As this example illustrates, sometimes the default behavior with
+destination directories is not exactly
what is wanted, so these commands support the following options to
allow more fine-grained control:
diff --git a/src/cp.c b/src/cp.c
index 6949a67..f84eac8 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -33,6 +33,7 @@
#include "ignore-value.h"
#include "quote.h"
#include "stat-time.h"
+#include "targetdir.h"
#include "utimens.h"
#include "acl.h"
@@ -567,6 +568,20 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
return true;
}
+/* Store FILE's status into *ST. If FILE does not exist, set *NEW_DST.
+ If there is some other error, report it and exit. */
+
+static void
+stat_target_operand (char const *file, struct stat *st, bool *new_dst)
+{
+ if (stat (file, st) != 0)
+ {
+ if (errno != ENOENT)
+ die (EXIT_FAILURE, errno, _("failed to access %s"), quoteaf (file));
+ *new_dst = true;
+ }
+}
+
/* FILE is the last operand of this command.
Return true if FILE is a directory.
But report an error and exit if there is a problem accessing FILE,
@@ -577,17 +592,17 @@ make_dir_parents_private (char const *const_dir, size_t src_offset,
Otherwise, set *NEW_DST. */
static bool
-target_directory_operand (char const *file, struct stat *st, bool *new_dst)
+target_directory_operand (char *file, struct stat *st, bool *new_dst)
{
- int err = (stat (file, st) == 0 ? 0 : errno);
- bool is_a_dir = !err && S_ISDIR (st->st_mode);
- if (err)
- {
- if (err != ENOENT)
- die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
- *new_dst = true;
- }
- return is_a_dir;
+ stat_target_operand (file, st, new_dst);
+ if (*new_dst || ! S_ISDIR (st->st_mode))
+ return false;
+ enum targetdir ty = targetdir_operand_type (file, NULL);
+ if (ty == TARGETDIR_VULNERABLE && ! getenv ("POSIXLY_CORRECT"))
+ die (EXIT_FAILURE, 0,
+ _("vulnerable target directory %s (append '/' to use anyway)"),
+ quoteaf (file));
+ return ty != TARGETDIR_NOT;
}
/* Scan the arguments, and copy each by calling copy.
@@ -623,7 +638,7 @@ do_copy (int n_files, char **file, const char *target_directory,
usage (EXIT_FAILURE);
}
/* Update NEW_DST and SB, which may be checked below. */
- ignore_value (target_directory_operand (file[n_files -1], &sb, &new_dst));
+ stat_target_operand (file[n_files -1], &sb, &new_dst);
}
else if (!target_directory)
{
diff --git a/src/install.c b/src/install.c
index 5b68261..6756667 100644
--- a/src/install.c
+++ b/src/install.c
@@ -42,6 +42,7 @@
#include "savewd.h"
#include "selinux.h"
#include "stat-time.h"
+#include "targetdir.h"
#include "utimens.h"
#include "xstrtol.h"
@@ -395,20 +396,29 @@ setdefaultfilecon (char const *file)
directory if it referred to anything at all. */
static bool
-target_directory_operand (char const *file)
+target_directory_operand (char *file)
{
- char const *b = last_component (file);
- size_t blen = strlen (b);
- bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
struct stat st;
- int err = (stat (file, &st) == 0 ? 0 : errno);
- bool is_a_dir = !err && S_ISDIR (st.st_mode);
- if (err && err != ENOENT)
- die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
- if (is_a_dir < looks_like_a_dir)
- die (EXIT_FAILURE, err, _("target %s is not a directory"),
+ if (stat (file, &st) != 0)
+ {
+ int err = errno;
+ if (err == ENOENT)
+ {
+ char const *b = last_component (file);
+ size_t blen = strlen (b);
+ if (blen != 0 && ! ISSLASH (b[blen - 1]))
+ return false;
+ }
+ die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
+ }
+ if (! S_ISDIR (st.st_mode))
+ return false;
+ enum targetdir ty = targetdir_operand_type (file, NULL);
+ if (ty == TARGETDIR_VULNERABLE)
+ die (EXIT_FAILURE, 0,
+ _("vulnerable target directory %s (append '/' to use anyway)"),
quoteaf (file));
- return is_a_dir;
+ return ty != TARGETDIR_NOT;
}
/* Report that directory DIR was made, if OPTIONS requests this. */
diff --git a/src/ln.c b/src/ln.c
index e86f581..122be26 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -32,6 +32,7 @@
#include "hash-triple.h"
#include "relpath.h"
#include "same.h"
+#include "targetdir.h"
#include "yesno.h"
#include "canonicalize.h"
@@ -120,22 +121,33 @@ errno_nonexisting (int err)
directory if it referred to anything at all. */
static bool
-target_directory_operand (char const *file)
+target_directory_operand (char *file)
{
- char const *b = last_component (file);
- size_t blen = strlen (b);
- bool looks_like_a_dir = (blen == 0 || ISSLASH (b[blen - 1]));
struct stat st;
int stat_result =
(dereference_dest_dir_symlinks ? stat (file, &st) : lstat (file, &st));
- int err = (stat_result == 0 ? 0 : errno);
- bool is_a_dir = !err && S_ISDIR (st.st_mode);
- if (err && ! errno_nonexisting (errno))
- die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
- if (is_a_dir < looks_like_a_dir)
- die (EXIT_FAILURE, err, _("target %s is not a directory"),
+ if (stat_result != 0)
+ {
+ int err = errno;
+ if (! errno_nonexisting (err))
+ die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
+ char const *b = last_component (file);
+ size_t blen = strlen (b);
+ if (blen == 0 || ISSLASH (b[blen - 1]))
+ die (EXIT_FAILURE, err, _("target %s is not a directory"),
+ quoteaf (file));
+ return false;
+ }
+ if (! S_ISDIR (st.st_mode))
+ return false;
+ enum targetdir ty
+ = targetdir_operand_type (file,
+ dereference_dest_dir_symlinks ? NULL : &st);
+ if (ty == TARGETDIR_VULNERABLE && ! getenv ("POSIXLY_CORRECT"))
+ die (EXIT_FAILURE, 0,
+ _("%s: vulnerable target directory (append '/' to use anyway)"),
quoteaf (file));
- return is_a_dir;
+ return ty != TARGETDIR_NOT;
}
/* Return FROM represented as relative to the dir of TARGET.
diff --git a/src/local.mk b/src/local.mk
index 1cb6859..e0d66b7 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -59,6 +59,7 @@ noinst_HEADERS = \
src/remove.h \
src/set-fields.h \
src/system.h \
+ src/targetdir.h \
src/uname.h
EXTRA_DIST += \
@@ -344,8 +345,8 @@ copy_sources = \
transform = s/ginstall/install/;/libstdbuf/!$(program_transform_name)
-src_ginstall_SOURCES = src/install.c src/prog-fprintf.c $(copy_sources) \
- $(selinux_sources)
+src_ginstall_SOURCES = src/install.c src/prog-fprintf.c src/targetdir.c \
+ $(copy_sources) $(selinux_sources)
# This is for the '[' program. Automake transliterates '[' and '/' to '_'.
src___SOURCES = src/lbracket.c
@@ -353,7 +354,7 @@ src___SOURCES = src/lbracket.c
nodist_src_coreutils_SOURCES = src/coreutils.h
src_coreutils_SOURCES = src/coreutils.c
-src_cp_SOURCES = src/cp.c $(copy_sources) $(selinux_sources)
+src_cp_SOURCES = src/cp.c src/targetdir.c $(copy_sources) $(selinux_sources)
src_dir_SOURCES = src/ls.c src/ls-dir.c
src_vdir_SOURCES = src/ls.c src/ls-vdir.c
src_id_SOURCES = src/id.c src/group-list.c
@@ -361,14 +362,16 @@ src_groups_SOURCES = src/groups.c src/group-list.c
src_ls_SOURCES = src/ls.c src/ls-ls.c
src_ln_SOURCES = src/ln.c \
src/force-link.c src/force-link.h \
- src/relpath.c src/relpath.h
+ src/relpath.c src/relpath.h \
+ src/targetdir.c
src_chown_SOURCES = src/chown.c src/chown-core.c
src_chgrp_SOURCES = src/chgrp.c src/chown-core.c
src_kill_SOURCES = src/kill.c src/operand2sig.c
src_realpath_SOURCES = src/realpath.c src/relpath.c src/relpath.h
src_timeout_SOURCES = src/timeout.c src/operand2sig.c
-src_mv_SOURCES = src/mv.c src/remove.c $(copy_sources) $(selinux_sources)
+src_mv_SOURCES = src/mv.c src/remove.c src/targetdir.c \
+ $(copy_sources) $(selinux_sources)
src_rm_SOURCES = src/rm.c src/remove.c
src_mkdir_SOURCES = src/mkdir.c src/prog-fprintf.c $(selinux_sources)
diff --git a/src/mv.c b/src/mv.c
index fc1fca4..6df4c5d 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -32,6 +32,7 @@
#include "filenamecat.h"
#include "remove.h"
#include "root-dev-ino.h"
+#include "targetdir.h"
#include "priv-set.h"
/* The official name of this program (e.g., no 'g' prefix). */
@@ -148,14 +149,23 @@ cp_option_init (struct cp_options *x)
than nonexistence (errno == ENOENT). */
static bool
-target_directory_operand (char const *file)
+target_directory_operand (char *file)
{
struct stat st;
- int err = (stat (file, &st) == 0 ? 0 : errno);
- bool is_a_dir = !err && S_ISDIR (st.st_mode);
- if (err && err != ENOENT)
- die (EXIT_FAILURE, err, _("failed to access %s"), quoteaf (file));
- return is_a_dir;
+ if (stat (file, &st) == 0)
+ {
+ if (! S_ISDIR (st.st_mode))
+ return false;
+ enum targetdir ty = targetdir_operand_type (file, NULL);
+ if (ty == TARGETDIR_VULNERABLE && ! getenv ("POSIXLY_CORRECT"))
+ die (EXIT_FAILURE, 0,
+ _("vulnerable target directory %s (append / to use anyway)"),
+ quoteaf (file));
+ return ty != TARGETDIR_NOT;
+ }
+ if (errno != ENOENT)
+ die (EXIT_FAILURE, errno, _("failed to access %s"), quoteaf (file));
+ return false;
}
/* Move SOURCE onto DEST. Handles cross-file-system moves.
diff --git a/src/targetdir.c b/src/targetdir.c
new file mode 100644
index 0000000..e1ef16e
--- /dev/null
+++ b/src/targetdir.c
@@ -0,0 +1,100 @@
+/* Check target directories for commands like cp and mv.
+ Copyright 2017 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Written by Paul Eggert. */
+
+#include <config.h>
+#include <targetdir.h>
+
+#include <die.h>
+#include <dirname.h>
+#include <root-uid.h>
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+/* Check whether DIR, which the caller presumably has already verified
+ is a directory or a symlink to a directory, is likely to be
+ vulnerable as the target directory of a command like 'cp ... DIR'.
+ If DIR_LSTAT is nonnull, it is the result of calling lstat on DIR.
+
+ Return TARGETDIR_OK if DIR is OK, which does not necessarily mean
+ that DIR is a directory or that it is invulnerable to the attack,
+ only that it satisfies the heuristics. Return TARGETDIR_NOT if DIR
+ becomes inaccessible or a non-directory while checking things.
+ Return TARGETDIR_VULNERABLE if the heuristics suggest that DIR is a
+ likely candidate to be hijacked by a symlink attack.
+
+ This function might temporarily modify the DIR string; it restores
+ the string to its original value before returning. */
+
+enum targetdir
+targetdir_operand_type (char *restrict dir,
+ struct stat const *restrict dir_lstat)
+{
+ char *lc = last_component (dir);
+ size_t lclen = strlen (lc);
+
+ /* If DIR ends in / or has a last component of . or .. then it is
+ good enough. */
+ if (lclen == 0 || ISSLASH (lc[lclen - 1])
+ || strcmp (lc, ".") == 0 || strcmp (lc, "..") == 0)
+ return TARGETDIR_OK;
+
+ char lc0 = *lc;
+ *lc = '\0';
+ struct stat parent_stat;
+ bool parent_stat_ok = stat (*dir ? dir : ".", &parent_stat) == 0;
+ *lc = lc0;
+
+ /* If DIR's parent cannot be statted, DIR can't be statted either. */
+ if (! parent_stat_ok)
+ return TARGETDIR_NOT;
+
+ uid_t euid = geteuid ();
+ if (parent_stat.st_uid == ROOT_UID || parent_stat.st_uid == euid)
+ {
+ /* If no other non-root user can write the parent directory, it
+ is safe. If the parent directory's UID and GID are the same,
+ assume the common convention of a single-user group with the
+ same ID, to avoid returning TARGETDIR_VULNERABLE when users
+ employing this convention have mode-775 directories. */
+ if (! (parent_stat.st_mode
+ & (S_IWOTH
+ | (parent_stat.st_uid == parent_stat.st_gid ? 0 : S_IWGRP))))
+ return TARGETDIR_OK;
+
+ /* If the parent is sticky, and no other non-root user owns
+ either the parent or DIR, it should be OK. Do not follow
+ symlinks when checking DIR for this. */
+ if (parent_stat.st_mode & S_ISVTX)
+ {
+ struct stat st;
+ if (!dir_lstat)
+ {
+ if (lstat (dir, &st) != 0)
+ return TARGETDIR_NOT;
+ dir_lstat = &st;
+ }
+ if (dir_lstat->st_uid == ROOT_UID || dir_lstat->st_uid == euid)
+ return TARGETDIR_OK;
+ }
+ }
+
+ return TARGETDIR_VULNERABLE;
+}
diff --git a/src/targetdir.h b/src/targetdir.h
new file mode 100644
index 0000000..29ea867
--- /dev/null
+++ b/src/targetdir.h
@@ -0,0 +1,4 @@
+struct stat;
+enum targetdir { TARGETDIR_VULNERABLE = -1, TARGETDIR_NOT, TARGETDIR_OK };
+enum targetdir targetdir_operand_type (char *restrict,
+ struct stat const *restrict);
diff --git a/tests/local.mk b/tests/local.mk
index 732ec99..d9d3350 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -130,6 +130,7 @@ all_root_tests = \
tests/mkdir/smack-root.sh \
tests/mv/hardlink-case.sh \
tests/mv/sticky-to-xpart.sh \
+ tests/mv/vulnerable-target.sh \
tests/rm/fail-2eperm.sh \
tests/rm/no-give-up.sh \
tests/rm/one-file-system.sh \
diff --git a/tests/mv/vulnerable-target.sh b/tests/mv/vulnerable-target.sh
new file mode 100755
index 0000000..d3b99ce
--- /dev/null
+++ b/tests/mv/vulnerable-target.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+# Check that mv diagnoses vulnerable target directories.
+
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ mv
+
+unset POSIXLY_CORRECT
+
+mkdir -m a+rwx risky || framework_failure_
+mkdir risky/d || framework_failure_
+echo foo >foo || framework_failure_
+
+mv foo risky/d && fail=1
+mv foo risky/d/ || fail=1
+mv risky/d/foo . || fail=1
+mv -t risky/d foo || fail=1
+mv risky/d/foo . || fail=1
+mv -T foo risky/d/foo || fail=1
+mv risky/d/foo . || fail=1
+POSIXLY_CORRECT=yes mv foo risky/d || fail=1
+mv risky/d/foo . || fail=1
+
+Exit $fail
--
2.7.4
