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 829b8c546362c4ce0d2c8563c3747f4d7bad2422..53789b5a4a1e351afdd4b6e05d1246f0fdf9db3d 100644 (file)
--- 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 987b46cb0e7e0c8447fa3a6969cd4893d91577fb..58280bae52e27f6de8dcf70292e5bc7b896cd968 100644 (file)
--- 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 bb3fb3f976ecdee754af4ff03350ef49f7c55830..f22f7ac7b9a0d6a669d911444bd549e2b6ce9b02 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 fcbc32b73865a64106a55d1aab5421552d470dfe..ce14a82ed96bd013ea26b23f70f148596f2ff113 100644 (file)
--- 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 9176d9b5a16a17ca682156773811512c43cf3ee5..c641580b0381bf469d4087d6bb067311daf198ba 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 14f6b96c7d6785540110d7db67447303398a3e3d..495e5c2405045f37bbdb05e72ec4f7012777c8ec 100644 (file)
--- 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);