From 063bb0250defafcc55544474a2961ecbc153882e Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Thu, 8 Jan 2015 15:39:49 +0000 Subject: [PATCH] Fix memory access violations exposed by running strip on fuzzed binaries. PR binutils/17512 * coffcode.h (coff_slurp_symbol_table): Return false if we failed to load the line table. * elf.c (_bfd_elf_map_sections_to_segments): Enforce a minimum maxpagesize of 1. * peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Fail if the Data Directory Size is too large. * objcopy.c (copy_object): Free the symbol table if no symbols could be loaded. (copy_file): Use bfd_close_all_done to close files that could not be copied. --- bfd/ChangeLog | 10 ++++++++++ bfd/coffcode.h | 7 ++++--- bfd/elf.c | 5 +++++ bfd/peXXigen.c | 10 ++++++++++ binutils/ChangeLog | 5 +++++ binutils/objcopy.c | 14 +++++++++++++- 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 3483d79031e..b6151cc3357 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,13 @@ +2015-01-08 Nick Clifton + + PR binutils/17512 + * coffcode.h (coff_slurp_symbol_table): Return false if we failed + to load the line table. + * elf.c (_bfd_elf_map_sections_to_segments): Enforce a minimum + maxpagesize of 1. + * peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Fail if + the Data Directory Size is too large. + 2015-01-06 H.J. Lu PR binutils/17512 diff --git a/bfd/coffcode.h b/bfd/coffcode.h index 695497f4b3f..9e1c20acf38 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -5012,13 +5012,13 @@ coff_slurp_symbol_table (bfd * abfd) #if defined(TIC80COFF) || defined(TICOFF) case C_UEXT: /* Tentative external definition. */ #endif - case C_EXTLAB: /* External load time label. */ - case C_HIDDEN: /* Ext symbol in dmert public lib. */ default: (*_bfd_error_handler) (_("%B: Unrecognized storage class %d for %s symbol `%s'"), abfd, src->u.syment.n_sclass, dst->symbol.section->name, dst->symbol.name); + case C_EXTLAB: /* External load time label. */ + case C_HIDDEN: /* Ext symbol in dmert public lib. */ dst->symbol.flags = BSF_DEBUGGING; dst->symbol.value = (src->u.syment.n_value); break; @@ -5046,7 +5046,8 @@ coff_slurp_symbol_table (bfd * abfd) p = abfd->sections; while (p) { - coff_slurp_line_table (abfd, p); + if (! coff_slurp_line_table (abfd, p)) + return FALSE; p = p->next; } } diff --git a/bfd/elf.c b/bfd/elf.c index f262cc3c7da..0aa5f8e4791 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -4011,6 +4011,11 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info) last_size = 0; phdr_index = 0; maxpagesize = bed->maxpagesize; + /* PR 17512: file: c8455299. + Avoid divide-by-zero errors later on. + FIXME: Should we abort if the maxpagesize is zero ? */ + if (maxpagesize == 0) + maxpagesize = 1; writable = FALSE; dynsec = bfd_get_section_by_name (abfd, ".dynamic"); if (dynsec != NULL diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index 09adf8348d7..0abe609f2e8 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -2930,6 +2930,16 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd) struct external_IMAGE_DEBUG_DIRECTORY *dd = (struct external_IMAGE_DEBUG_DIRECTORY *)(data + (addr - section->vma)); + /* PR 17512: file: 0f15796a. */ + if (ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size + (addr - section->vma) + > bfd_get_section_size (section)) + { + _bfd_error_handler (_("%A: Data Directory size (%lx) exceeds space left in section (%lx)"), + obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size, + bfd_get_section_size (section) - (addr - section->vma)); + return FALSE; + } + for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++) { diff --git a/binutils/ChangeLog b/binutils/ChangeLog index d6c3070eb6c..e6fa3c11e57 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,6 +1,11 @@ 2015-01-08 Nick Clifton PR binutils/17512 + * ojcopy.c (copy_object): Free the symbol table if no symbols + could be loaded. + (copy_file): Use bfd_close_all_done to close files that could not + be copied. + * sysdump.c (getINT): Fail if reading off the end of the buffer. Replace call to abort with a call to fatal. (getCHARS): Prevetn reading off the end of the buffer. diff --git a/binutils/objcopy.c b/binutils/objcopy.c index 25f0131e9bc..9524bb85de5 100644 --- a/binutils/objcopy.c +++ b/binutils/objcopy.c @@ -1776,6 +1776,14 @@ copy_object (bfd *ibfd, bfd *obfd, const bfd_arch_info_type *input_arch) bfd_nonfatal_message (NULL, ibfd, NULL, NULL); return FALSE; } + /* PR 17512: file: d6323821 + If the symbol table could not be loaded do not pretend that we have + any symbols. This trips us up later on when we load the relocs. */ + if (symcount == 0) + { + free (isympp); + osympp = isympp = NULL; + } /* BFD mandates that all output sections be created and sizes set before any output is done. Thus, we traverse all sections multiple times. */ @@ -2552,7 +2560,11 @@ copy_file (const char *input_filename, const char *output_filename, if (! copy_object (ibfd, obfd, input_arch)) status = 1; - if (!bfd_close (obfd)) + /* PR 17512: file: 0f15796a. + If the file could not be copied it may not be in a writeable + state. So use bfd_close_all_done to avoid the possibility of + writing uninitialised data into the file. */ + if (! (status ? bfd_close_all_done (obfd) : bfd_close (obfd))) { status = 1; bfd_nonfatal_message (output_filename, NULL, NULL, NULL); -- 2.30.2