+2015-01-22 Andreas Gruenbacher <agruen@gnu.org>
+
+ Fix the fix for CVE-2015-1196
+ * src/util.c (filename_is_safe): New function split off from name_is_valid().
+ (symlink_target_is_valid): Explain why we cannot have absolute symlinks or
+ symlinks with ".." components for now.
+ (move_file): Move absolute filename check here and explain.
+ * tests/symlinks: Put test case with ".." symlink in comments for now.
+ * NEWS: Add CVE number.
+
+2015-01-21 Andreas Gruenbacher <agruen@gnu.org>
+
+ For renames and copies, make sure that both file names are valid
+ * src/patch.c (main): Allow there_is_another_patch() to set the
+ skip_rest_of_patch flag.
+ * src/pch.c (intuit_diff_type): For renames and copies, also check the "other"
+ file name.
+ (pch_copy, pch_rename): Now that both names are checked in intuit_diff_type(),
+ we know they are defined here.
+
2015-01-20 Andreas Gruenbacher <agruen@gnu.org>
Fail when out of memory in set_hunkmax()
deleting".
* Function names in hunks (from diff -p) are now preserved in reject files.
* With git-style patches, symlinks that point outside the working directory
- will no longer be created.
+ will no longer be created (CVE-2015-1196).
Changes in version 2.7.1:
#! /bin/sh
# Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for GNU patch 2.7.2.
+# Generated by GNU Autoconf 2.69 for GNU patch 2.7.3.
#
# Report bugs to <bug-patch@gnu.org>.
#
# Identity of this package.
PACKAGE_NAME='GNU patch'
PACKAGE_TARNAME='patch'
-PACKAGE_VERSION='2.7.2'
-PACKAGE_STRING='GNU patch 2.7.2'
+PACKAGE_VERSION='2.7.3'
+PACKAGE_STRING='GNU patch 2.7.3'
PACKAGE_BUGREPORT='bug-patch@gnu.org'
PACKAGE_URL='http://www.gnu.org/software/patch/'
# Omit some internal or obsolete options to make the list less imposing.
# This message is too long to be a string in the A/UX 3.1 sh.
cat <<_ACEOF
-\`configure' configures GNU patch 2.7.2 to adapt to many kinds of systems.
+\`configure' configures GNU patch 2.7.3 to adapt to many kinds of systems.
Usage: $0 [OPTION]... [VAR=VALUE]...
if test -n "$ac_init_help"; then
case $ac_init_help in
- short | recursive ) echo "Configuration of GNU patch 2.7.2:";;
+ short | recursive ) echo "Configuration of GNU patch 2.7.3:";;
esac
cat <<\_ACEOF
test -n "$ac_init_help" && exit $ac_status
if $ac_init_version; then
cat <<\_ACEOF
-GNU patch configure 2.7.2
+GNU patch configure 2.7.3
generated by GNU Autoconf 2.69
Copyright (C) 2012 Free Software Foundation, Inc.
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.
-It was created by GNU patch $as_me 2.7.2, which was
+It was created by GNU patch $as_me 2.7.3, which was
generated by GNU Autoconf 2.69. Invocation command line was
$ $0 $@
# Define the identity of the package.
PACKAGE='patch'
- VERSION='2.7.2'
+ VERSION='2.7.3'
cat >>confdefs.h <<_ACEOF
# report actual input values of CONFIG_FILES etc. instead of their
# values after options handling.
ac_log="
-This file was extended by GNU patch $as_me 2.7.2, which was
+This file was extended by GNU patch $as_me 2.7.3, which was
generated by GNU Autoconf 2.69. Invocation command line was
CONFIG_FILES = $CONFIG_FILES
cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
ac_cs_version="\\
-GNU patch config.status 2.7.2
+GNU patch config.status 2.7.3
configured by $0, generated by GNU Autoconf 2.69,
with options \\"\$ac_cs_config\\"
bool mismatch = false;
char const *outname = NULL;
+ if (skip_rest_of_patch)
+ somefailed = true;
+
if (have_git_diff != pch_git_diff ())
{
if (have_git_diff)
return false;
}
- if (IS_ABSOLUTE_FILE_NAME (name))
- is_valid = false;
- else
- for (n = name; *n; )
- {
- if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n)))
- {
- is_valid = false;
- break;
- }
- while (*n && ! ISSLASH (*n))
- n++;
- while (ISSLASH (*n))
- n++;
- }
+ is_valid = filename_is_safe (name);
/* Allow any filename if we are in the filesystem root. */
if (! is_valid && cwd_is_root (name))
}
}
+ if ((pch_rename () || pch_copy ())
+ && ! inname
+ && ! ((i == OLD || i == NEW) &&
+ p_name[! reverse] &&
+ name_is_valid (p_name[! reverse])))
+ {
+ say ("Cannot %s file without two valid file names\n", pch_rename () ? "rename" : "copy");
+ skip_rest_of_patch = true;
+ }
+
if (i == NONE)
{
if (inname)
bool pch_copy (void)
{
- return p_copy[OLD] && p_copy[NEW]
- && p_name[OLD] && p_name[NEW];
+ return p_copy[OLD] && p_copy[NEW];
}
bool pch_rename (void)
{
- return p_rename[OLD] && p_rename[NEW]
- && p_name[OLD] && p_name[NEW];
+ return p_rename[OLD] && p_rename[NEW];
}
/* Return the specified line position in the old file of the old context. */
}
}
+/* 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;
-
- if (IS_ABSOLUTE_FILE_NAME (to))
- is_valid = true;
- else if (IS_ABSOLUTE_FILE_NAME (target))
- is_valid = false;
- else
- {
- unsigned int depth = 0;
- char const *t;
-
- is_valid = true;
- t = to;
- while (*t)
- {
- while (*t && ! ISSLASH (*t))
- t++;
- if (ISSLASH (*t))
- {
- while (ISSLASH (*t))
- t++;
- depth++;
- }
- }
-
- t = target;
- while (*t)
- {
- if (*t == '.' && *++t == '.' && (! *++t || ISSLASH (*t)))
- {
- if (! depth--)
- {
- is_valid = false;
- break;
- }
- }
- else
- {
- while (*t && ! ISSLASH (*t))
- t++;
- depth++;
- }
- while (ISSLASH (*t))
- t++;
- }
- }
+ 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);
read_fatal ();
buffer[size] = 0;
- if (! symlink_target_is_valid (buffer, to))
+ /* 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);
return xstat (filename, st) == 0 ? 0 : errno;
}
+/* Check if a filename is relative and free of ".." components.
+ Such a path cannot lead to files outside the working tree
+ as long as the working tree only contains symlinks that are
+ "filename_is_safe" when followed. */
+bool
+filename_is_safe (char const *name)
+{
+ if (IS_ABSOLUTE_FILE_NAME (name))
+ return false;
+ while (*name)
+ {
+ if (*name == '.' && *++name == '.'
+ && ( ! *++name || ISSLASH (*name)))
+ return false;
+ while (*name && ! ISSLASH (*name))
+ name++;
+ while (ISSLASH (*name))
+ name++;
+ }
+ return true;
+}
+
/* Check if we are in the root of a particular filesystem namespace ("/" on
UNIX or a particular drive's root on DOS-like systems). */
bool
void set_queued_output (struct stat const *, bool);
bool has_queued_output (struct stat const *);
int stat_file (char const *, struct stat *);
+bool filename_is_safe (char const *);
bool cwd_is_root (char const *);
enum file_attributes {
# Patch should not create symlinks which point outside the working directory.
-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
+# We cannot even ensure that symlinks with ".." components are safe: we cannot
+# 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 > bad-symlink-target1.diff <<EOF
diff --git a/bar b/bar