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.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 7c3b581..6a04543 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,6 +1,18 @@
+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.
+
 2014-11-05  Nick Clifton  <nickc@redhat.com>
 
-       PR binutils/15731
+       PR binutils/17531
        * readelf.c (printable_section_name): New function.
        (printable_section_name_from_index): New function.
        (dump_relocations): Use new function.
@@ -22,7 +34,7 @@
 
 2014-11-05  Nick Clifton  <nickc@redhat.com>
 
-       PR binutils/15733
+       PR binutils/17533
        * bucomm.c (is_valid_archive_path): New function.
        * bucomm.h (is_valid_archive_path): Prototype it.
        * ar.c (extract_file): Call is_valid_archive_path to verify a

diff --git a/binutils/ar.c b/binutils/ar.c
index ebd9528..117826d 100644 (file)
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -1034,6 +1034,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 fd73070..b8deff5 100644 (file)
--- 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 a93c378..a71a8fb 100644 (file)
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -21,6 +21,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 *);
 
@@ -56,20 +58,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 eee77b1..39eb1d2 100644 (file)
--- 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 3b353ad..8454bc6 100644 (file)
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -2295,6 +2295,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);