diff options
author | Nick Clifton <nickc@redhat.com> | 2014-11-06 14:49:10 +0000 |
---|---|---|
committer | Nick Clifton <nickc@redhat.com> | 2014-11-06 14:49:10 +0000 |
commit | dd9b91de2149ee81d47f708e7b0bbf57da10ad42 (patch) | |
tree | 9abc44b553f584ed318e6747e7f49d1095ea13c6 | |
parent | 834107255bbefceb445fa733ebc1ea5d9f41ec7f (diff) | |
download | gdb-dd9b91de2149ee81d47f708e7b0bbf57da10ad42.zip gdb-dd9b91de2149ee81d47f708e7b0bbf57da10ad42.tar.gz gdb-dd9b91de2149ee81d47f708e7b0bbf57da10ad42.tar.bz2 |
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.
-rw-r--r-- | binutils/ChangeLog | 16 | ||||
-rw-r--r-- | binutils/ar.c | 9 | ||||
-rw-r--r-- | binutils/bucomm.c | 26 | ||||
-rw-r--r-- | binutils/bucomm.h | 12 | ||||
-rw-r--r-- | binutils/doc/binutils.texi | 3 | ||||
-rw-r--r-- | binutils/objcopy.c | 6 |
6 files changed, 65 insertions, 7 deletions
diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 7c3b581..6a04543 100644 --- 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 --- 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); |