Attachment 7899 Details for Bug 17552 – Proposed patch

[patch] Proposed patch

ar.path.patch (text/plain), 3.61 KB, created by Nick Clifton on 2014-11-05 11:48:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clifton

Created: 2014-11-05 11:48:34 UTC

Size: 3.61 KB

patch

obsolete

>diff --git a/binutils/ar.c b/binutils/ar.c
>index ebd9528..117826d 100644
>--- 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
>--- 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
>--- 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
>--- 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
>--- 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);

Actions: View | Diff

Attachments on bug 17552: 7899 | 7909 | 7913