polynomial-c 15/01/23 19:02:06
Added: patch-2.7.2-fix_for_CVE-2015-1196_fix.patch
patch-2.7.2-valid_filenames_on_renames_and_copies.patch
Log:
Revbump to add two upstream fixes
(Portage version: 2.2.15/cvs/Linux x86_64, signed Manifest commit with key
0x981CA6FC)
Revision Changes Path
1.1
sys-devel/patch/files/patch-2.7.2-fix_for_CVE-2015-1196_fix.patch
file :
http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-devel/patch/files/patch-2.7.2-fix_for_CVE-2015-1196_fix.patch?rev=1.1&view=markup
plain:
http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-devel/patch/files/patch-2.7.2-fix_for_CVE-2015-1196_fix.patch?rev=1.1&content-type=text/plain
Index: patch-2.7.2-fix_for_CVE-2015-1196_fix.patch
===================================================================
>From 41688ad8ef88bc296f3bed30b171ec73e5876b88 Mon Sep 17 00:00:00 2001
From: Andreas Gruenbacher <agr...@gnu.org>
Date: Wed, 21 Jan 2015 09:01:15 +0000
Subject: 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.
---
diff --git a/NEWS b/NEWS
index d3f1c2d..d79cead 100644
--- a/NEWS
+++ b/NEWS
@@ -4,7 +4,7 @@
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:
diff --git a/src/pch.c b/src/pch.c
index bb39576..028d51f 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -401,21 +401,7 @@ name_is_valid (char const *name)
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))
diff --git a/src/util.c b/src/util.c
index 94c7582..ae05caa 100644
--- a/src/util.c
+++ b/src/util.c
@@ -423,55 +423,18 @@ 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;
-
- 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);
@@ -520,7 +483,11 @@ move_file (char const *from, bool *from_needs_removal,
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);
@@ -1720,6 +1687,28 @@ int stat_file (char const *filename, struct stat *st)
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
diff --git a/src/util.h b/src/util.h
index 579c5de..6b3308a 100644
--- a/src/util.h
+++ b/src/util.h
@@ -69,6 +69,7 @@ enum file_id_type lookup_file_id (struct stat const *);
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 {
diff --git a/tests/symlinks b/tests/symlinks
index 6211026..04a9b73 100644
--- a/tests/symlinks
+++ b/tests/symlinks
@@ -148,20 +148,24 @@ ncheck 'test ! -L symlink'
# 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
--
cgit v0.9.0.2
1.1
sys-devel/patch/files/patch-2.7.2-valid_filenames_on_renames_and_copies.patch
file :
http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-devel/patch/files/patch-2.7.2-valid_filenames_on_renames_and_copies.patch?rev=1.1&view=markup
plain:
http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-devel/patch/files/patch-2.7.2-valid_filenames_on_renames_and_copies.patch?rev=1.1&content-type=text/plain
Index: patch-2.7.2-valid_filenames_on_renames_and_copies.patch
===================================================================
>From 17953b5893f7c9835f0dd2a704ba04e0371d2cbd Mon Sep 17 00:00:00 2001
From: Andreas Gruenbacher <agr...@gnu.org>
Date: Wed, 21 Jan 2015 12:01:08 +0000
Subject: 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.
---
diff --git a/src/patch.c b/src/patch.c
index 441732e..cb4dbb2 100644
--- a/src/patch.c
+++ b/src/patch.c
@@ -196,6 +196,9 @@ main (int argc, char **argv)
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)
diff --git a/src/pch.c b/src/pch.c
index 33facd9..bb39576 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -978,6 +978,16 @@ intuit_diff_type (bool need_header, mode_t *p_file_type)
}
}
+ 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)
@@ -2178,14 +2188,12 @@ pch_name (enum nametype type)
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. */
--
cgit v0.9.0.2
