Prevent archive memebers with illegal pathnames from being extracted from an archive.

	PR binutils/17552, binutils/17533
	* bucomm.c (is_valid_archive_path): New function.  Returns false
	for absolute pathnames and pathnames that include /../.
	* bucomm.h (is_valid_archive_path): Add prototype.
	* ar.c (extract_file): Use new function to check for valid
	pathnames when extracting files from an archive.
	* objcopy.c (copy_archive): Likewise.
	* doc/binutils.texi: Update documentation to mention the
	limitation on pathname of archive members.

6 files changed, 63 insertions, 5 deletions

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 829b8c5..53789b5 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,15 @@
+2014-11-06  Nick Clifton  <nickc@redhat.com>
+
+	PR binutils/17552, binutils/17533
+	* bucomm.c (is_valid_archive_path): New function.  Returns false
+	for absolute pathnames and pathnames that include /../.
+	* bucomm.h (is_valid_archive_path): Add prototype.
+	* ar.c (extract_file): Use new function to check for valid
+	pathnames when extracting files from an archive.
+	* objcopy.c (copy_archive): Likewise.
+	* doc/binutils.texi: Update documentation to mention the
+	limitation on pathname of archive members.
+
 2013-12-10  Roland McGrath  <mcgrathr@google.com>
 
 	* Makefile.am (install-exec-local): Prefix libtool invocation with
diff --git a/binutils/ar.c b/binutils/ar.c
index 987b46c..58280ba 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -1031,6 +1031,15 @@ extract_file (bfd *abfd)
   bfd_size_type size;
   struct stat buf;
 
+  /* PR binutils/17533: Do not allow directory traversal
+     outside of the current directory tree.  */
+  if (! is_valid_archive_path (bfd_get_filename (abfd)))
+    {
+      non_fatal (_("illegal pathname found in archive member: %s"),
+		 bfd_get_filename (abfd));
+      return;
+    }
+
   if (bfd_stat_arch_elt (abfd, &buf) != 0)
     /* xgettext:c-format */
     fatal (_("internal stat error on %s"), bfd_get_filename (abfd));
diff --git a/binutils/bucomm.c b/binutils/bucomm.c
index bb3fb3f..f22f7ac 100644
--- a/binutils/bucomm.c
+++ b/binutils/bucomm.c
@@ -624,3 +624,29 @@ bfd_get_archive_filename (const bfd *abfd)
 	   bfd_get_filename (abfd));
   return buf;
 }
+
+/* Returns TRUE iff PATHNAME, a filename of an archive member,
+   is valid for writing.  For security reasons absolute paths
+   and paths containing /../ are not allowed.  See PR 17533.  */
+
+bfd_boolean
+is_valid_archive_path (char const * pathname)
+{
+  const char * n = pathname;
+
+  if (IS_ABSOLUTE_PATH (n))
+    return FALSE;
+
+  while (*n)
+    {
+      if (*n == '.' && *++n == '.' && ( ! *++n || IS_DIR_SEPARATOR (*n)))
+	return FALSE;
+
+      while (*n && ! IS_DIR_SEPARATOR (*n))
+	n++;
+      while (IS_DIR_SEPARATOR (*n))
+	n++;
+    }
+
+  return TRUE;
+}
diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index fcbc32b..ce14a82 100644
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -23,6 +23,8 @@
 #ifndef _BUCOMM_H
 #define _BUCOMM_H
 
+/* In bucomm.c.  */
+
 /* Return the filename in a static buffer.  */
 const char *bfd_get_archive_filename (const bfd *);
 
@@ -58,20 +60,22 @@ bfd_vma parse_vma (const char *, const char *);
 
 off_t get_file_size (const char *);
 
+bfd_boolean is_valid_archive_path (char const *);
+
 extern char *program_name;
 
-/* filemode.c */
+/* In filemode.c.  */
 void mode_string (unsigned long, char *);
 
-/* version.c */
+/* In version.c.  */
 extern void print_version (const char *);
 
-/* rename.c */
+/* In rename.c.  */
 extern void set_times (const char *, const struct stat *);
 
 extern int smart_rename (const char *, const char *, int);
 
-/* libiberty.  */
+/* In libiberty.  */
 void *xmalloc (size_t);
 
 void *xrealloc (void *, size_t);
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 9176d9b..c641580 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -234,7 +234,8 @@ a normal archive.  Instead the elements of the first archive are added
 individually to the second archive.
 
 The paths to the elements of the archive are stored relative to the
-archive itself.
+archive itself.  For security reasons absolute paths and paths with a
+@code{/../} component are not allowed.
 
 @cindex compatibility, @command{ar}
 @cindex @command{ar} compatibility
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 14f6b96..495e5c2 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -2206,6 +2206,12 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
       bfd_boolean del = TRUE;
       bfd_boolean ok_object;
 
+      /* PR binutils/17533: Do not allow directory traversal
+	 outside of the current directory tree by archive members.  */
+      if (! is_valid_archive_path (bfd_get_filename (this_element)))
+	fatal (_("illegal pathname found in archive member: %s"),
+	       bfd_get_filename (this_element));
+
       /* Create an output file for this member.  */
       output_name = concat (dir, "/",
 			    bfd_get_filename (this_element), (char *) 0);