From 36e9d67b868c85232ab630514260f0d9c9c6b27b Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Mon, 10 Nov 2014 14:18:45 +0000 Subject: [PATCH] More fixes for problems exposed by valgrind and the address sanitizer when displaying the contents of corrupt files. PR binutils/17521 * coff-i386.c (NUM_HOWTOS): New define. (RTYPE2HOWTO): Use it. (coff_i386_rtype_to_howto): Likewise. (coff_i386_reloc_name_lookup): Likewise. (CALC_ADDEND): Check that reloc r_type field is valid. * coff-x86_64.c (NUM_HOWTOS): New define. (RTYPE2HOWTO): Use it. (coff_amd64_rtype_to_howto): Likewise. (coff_amd64_reloc_name_lookup): Likewise. (CALC_ADDEND): Check that reloc r_type field is valid. * coffcode.h (coff_slurp_line_table): Check for symbol table indexing underflow. (coff_slurp_symbol_table): Use zalloc to ensure that all table entries are initialised. * coffgen.c (_bfd_coff_read_string_table): Initialise unused bits in the string table. Also ensure that the table is 0 terminated. (coff_get_normalized_symtab): Check for symbol table indexing underflow. * opncls.c (bfd_alloc): Catch the case where a small negative size can result in only 1 byte being allocated. (bfd_alloc2): Use bfd_alloc. * pe-mips.c (NUM_HOWTOS): New define. (coff_mips_reloc_name_lookup): Use it. (CALC_ADDEND): Check that reloc r_type field is valid. * peXXigen.c (_bfd_XXi_swap_aouthdr_in): Initialise unused entries in the DataDirectory. (pe_print_idata): Avoid reading beyond the end of the data block wen printing strings. (pe_print_edata): Likewise. Check for table indexing underflow. * peicode.h (pe_mkobject): Initialise the pe_opthdr field. (pe_bfd_object_p): Allocate and initialize enough space to hold a PEAOUTHDR, even if the opt_hdr field specified less. --- bfd/ChangeLog | 37 +++++++++++++++++++++++++++++++++++++ bfd/coff-i386.c | 17 ++++++++++------- bfd/coff-x86_64.c | 11 +++++++---- bfd/coffcode.h | 9 +++------ bfd/coffgen.c | 17 +++++++++++++---- bfd/opncls.c | 26 +++++++++----------------- bfd/pe-mips.c | 9 +++++---- bfd/peXXigen.c | 33 +++++++++++++++++++++++++-------- bfd/peicode.h | 14 ++++++++++---- 9 files changed, 119 insertions(+), 54 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 23c8419..ab34855 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,40 @@ +2014-11-10 Nick Clifton + + PR binutils/17521 + * coff-i386.c (NUM_HOWTOS): New define. + (RTYPE2HOWTO): Use it. + (coff_i386_rtype_to_howto): Likewise. + (coff_i386_reloc_name_lookup): Likewise. + (CALC_ADDEND): Check that reloc r_type field is valid. + * coff-x86_64.c (NUM_HOWTOS): New define. + (RTYPE2HOWTO): Use it. + (coff_amd64_rtype_to_howto): Likewise. + (coff_amd64_reloc_name_lookup): Likewise. + (CALC_ADDEND): Check that reloc r_type field is valid. + * coffcode.h (coff_slurp_line_table): Check for symbol table + indexing underflow. + (coff_slurp_symbol_table): Use zalloc to ensure that all table + entries are initialised. + * coffgen.c (_bfd_coff_read_string_table): Initialise unused bits + in the string table. Also ensure that the table is 0 terminated. + (coff_get_normalized_symtab): Check for symbol table indexing + underflow. + * opncls.c (bfd_alloc): Catch the case where a small negative size + can result in only 1 byte being allocated. + (bfd_alloc2): Use bfd_alloc. + * pe-mips.c (NUM_HOWTOS): New define. + (coff_mips_reloc_name_lookup): Use it. + (CALC_ADDEND): Check that reloc r_type field is valid. + * peXXigen.c (_bfd_XXi_swap_aouthdr_in): Initialise unused entries + in the DataDirectory. + (pe_print_idata): Avoid reading beyond the end of the data block + wen printing strings. + (pe_print_edata): Likewise. + Check for table indexing underflow. + * peicode.h (pe_mkobject): Initialise the pe_opthdr field. + (pe_bfd_object_p): Allocate and initialize enough space to hold a + PEAOUTHDR, even if the opt_hdr field specified less. + 2014-11-08 Alan Modra * peXXigen.c (pe_print_idata): Revert last patch, cast lhs instead. diff --git a/bfd/coff-i386.c b/bfd/coff-i386.c index 87b014b..848d69b 100644 --- a/bfd/coff-i386.c +++ b/bfd/coff-i386.c @@ -340,16 +340,18 @@ static reloc_howto_type howto_table[] = PCRELOFFSET) /* pcrel_offset */ }; +#define NUM_HOWTOS (sizeof (howto_table) / sizeof (howto_table[0])) + /* Turn a howto into a reloc nunmber */ #define SELECT_RELOC(x,howto) { x.r_type = howto->type; } #define BADMAG(x) I386BADMAG(x) #define I386 1 /* Customize coffcode.h */ -#define RTYPE2HOWTO(cache_ptr, dst) \ - ((cache_ptr)->howto = \ - ((dst)->r_type < sizeof (howto_table) / sizeof (howto_table[0]) \ - ? howto_table + (dst)->r_type \ +#define RTYPE2HOWTO(cache_ptr, dst) \ + ((cache_ptr)->howto = \ + ((dst)->r_type < NUM_HOWTOS \ + ? howto_table + (dst)->r_type \ : NULL)) /* For 386 COFF a STYP_NOLOAD | STYP_BSS section is part of a shared @@ -386,7 +388,8 @@ static reloc_howto_type howto_table[] = cache_ptr->addend = - (ptr->section->vma + ptr->value); \ else \ cache_ptr->addend = 0; \ - if (ptr && howto_table[reloc.r_type].pc_relative) \ + if (ptr && reloc.r_type < NUM_HOWTOS \ + && howto_table[reloc.r_type].pc_relative) \ cache_ptr->addend += asect->vma; \ } @@ -438,7 +441,7 @@ coff_i386_rtype_to_howto (bfd *abfd ATTRIBUTE_UNUSED, { reloc_howto_type *howto; - if (rel->r_type >= sizeof (howto_table) / sizeof (howto_table[0])) + if (rel->r_type >= NUM_HOWTOS) { bfd_set_error (bfd_error_bad_value); return NULL; @@ -574,7 +577,7 @@ coff_i386_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED, { unsigned int i; - for (i = 0; i < sizeof (howto_table) / sizeof (howto_table[0]); i++) + for (i = 0; i < NUM_HOWTOS; i++) if (howto_table[i].name != NULL && strcasecmp (howto_table[i].name, r_name) == 0) return &howto_table[i]; diff --git a/bfd/coff-x86_64.c b/bfd/coff-x86_64.c index 8d23733..2a21bb8 100644 --- a/bfd/coff-x86_64.c +++ b/bfd/coff-x86_64.c @@ -448,6 +448,8 @@ static reloc_howto_type howto_table[] = PCRELOFFSET) /* pcrel_offset */ }; +#define NUM_HOWTOS ARRAY_SIZE (howto_table) + /* Turn a howto into a reloc nunmber */ #define SELECT_RELOC(x,howto) { x.r_type = howto->type; } @@ -456,7 +458,7 @@ static reloc_howto_type howto_table[] = #define RTYPE2HOWTO(cache_ptr, dst) \ ((cache_ptr)->howto = \ - ((dst)->r_type < ARRAY_SIZE (howto_table)) \ + ((dst)->r_type < NUM_HOWTOS) \ ? howto_table + (dst)->r_type \ : NULL) @@ -496,7 +498,8 @@ static reloc_howto_type howto_table[] = cache_ptr->addend = - (ptr->section->vma + ptr->value); \ else \ cache_ptr->addend = 0; \ - if (ptr && howto_table[reloc.r_type].pc_relative) \ + if (ptr && reloc.r_type < NUM_HOWTOS \ + && howto_table[reloc.r_type].pc_relative) \ cache_ptr->addend += asect->vma; \ } @@ -546,7 +549,7 @@ coff_amd64_rtype_to_howto (bfd *abfd ATTRIBUTE_UNUSED, { reloc_howto_type *howto; - if (rel->r_type >= ARRAY_SIZE (howto_table)) + if (rel->r_type >= NUM_HOWTOS) { bfd_set_error (bfd_error_bad_value); return NULL; @@ -689,7 +692,7 @@ coff_amd64_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED, { unsigned int i; - for (i = 0; i < sizeof (howto_table) / sizeof (howto_table[0]); i++) + for (i = 0; i < NUM_HOWTOS; i++) if (howto_table[i].name != NULL && strcasecmp (howto_table[i].name, r_name) == 0) return &howto_table[i]; diff --git a/bfd/coffcode.h b/bfd/coffcode.h index 3abb6a3..8c6b1dd 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -4032,8 +4032,6 @@ coff_write_object_contents (bfd * abfd) internal_f.f_flags |= F_DYNLOAD; #endif - memset (&internal_a, 0, sizeof internal_a); - /* Set up architecture-dependent stuff. */ { unsigned int magic = 0; @@ -4569,8 +4567,7 @@ coff_slurp_line_table (bfd *abfd, asection *asect) /* PR 17512 file: 078-10659-0.004 */ if (sym < obj_symbols (abfd) - || sym > obj_symbols (abfd) - + obj_raw_syment_count (abfd) * sizeof (coff_symbol_type)) + || sym > obj_symbols (abfd) + obj_raw_syment_count (abfd)) sym = NULL; cache_ptr->u.sym = (asymbol *) sym; @@ -4683,7 +4680,7 @@ coff_slurp_symbol_table (bfd * abfd) amt = obj_raw_syment_count (abfd); amt *= sizeof (unsigned int); - table_ptr = (unsigned int *) bfd_alloc (abfd, amt); + table_ptr = (unsigned int *) bfd_zalloc (abfd, amt); if (table_ptr == NULL) return FALSE; @@ -4697,8 +4694,8 @@ coff_slurp_symbol_table (bfd * abfd) { combined_entry_type *src = native_symbols + this_index; table_ptr[this_index] = number_of_symbols; - dst->symbol.the_bfd = abfd; + dst->symbol.the_bfd = abfd; dst->symbol.name = (char *) (src->u.syment._n._n_n._n_offset); /* We use the native name field to point to the cached field. */ src->u.syment._n._n_n._n_zeroes = (bfd_hostptr_t) dst; diff --git a/bfd/coffgen.c b/bfd/coffgen.c index 9ad0783..6c69902 100644 --- a/bfd/coffgen.c +++ b/bfd/coffgen.c @@ -1690,7 +1690,13 @@ _bfd_coff_read_string_table (bfd *abfd) return NULL; } - strings = (char *) bfd_malloc (strsize); + strings = (char *) bfd_malloc (strsize + 1); + /* PR 17521 file: 079-54929-0.004. + A corrupt file could contain an index that points into the first + STRING_SIZE_SIZE bytes of the string table, so make sure that + they are zero. */ + memset (strings, 0, STRING_SIZE_SIZE); + if (strings == NULL) return NULL; @@ -1703,7 +1709,8 @@ _bfd_coff_read_string_table (bfd *abfd) obj_coff_strings (abfd) = strings; obj_coff_strings_len (abfd) = strsize; - + /* Terminate the string table, just in case. */ + strings[strsize] = 0; return strings; } @@ -1884,7 +1891,8 @@ coff_get_normalized_symtab (bfd *abfd) if (string_table == NULL) return NULL; } - if (internal_ptr->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd)) + if (internal_ptr->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd) + || string_table + internal_ptr->u.syment._n._n_n._n_offset < string_table) internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) _(""); else internal_ptr->u.syment._n._n_n._n_offset = @@ -1901,7 +1909,8 @@ coff_get_normalized_symtab (bfd *abfd) { BFD_ASSERT (debug_sec != NULL); /* PR binutils/17512: Catch out of range offsets into the debug data. */ - if (internal_ptr->u.syment._n._n_n._n_offset > debug_sec->size) + if (internal_ptr->u.syment._n._n_n._n_offset > debug_sec->size + || debug_sec_data + internal_ptr->u.syment._n._n_n._n_offset < debug_sec_data) internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) _(""); else internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) diff --git a/bfd/opncls.c b/bfd/opncls.c index a2a35f4..a22fba0 100644 --- a/bfd/opncls.c +++ b/bfd/opncls.c @@ -937,14 +937,19 @@ void * bfd_alloc (bfd *abfd, bfd_size_type size) { void *ret; + unsigned long ul_size = (unsigned long) size; - if (size != (unsigned long) size) + if (size != ul_size + /* A small negative size can result in objalloc_alloc allocating just + 1 byte of memory, but the caller will be expecting more. So catch + this case here. */ + || (size != 0 && (((ul_size + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1)) == 0))) { bfd_set_error (bfd_error_no_memory); return NULL; } - - ret = objalloc_alloc ((struct objalloc *) abfd->memory, (unsigned long) size); + + ret = objalloc_alloc ((struct objalloc *) abfd->memory, ul_size); if (ret == NULL) bfd_set_error (bfd_error_no_memory); return ret; @@ -965,8 +970,6 @@ DESCRIPTION void * bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size) { - void *ret; - if ((nmemb | size) >= HALF_BFD_SIZE_TYPE && size != 0 && nmemb > ~(bfd_size_type) 0 / size) @@ -975,18 +978,7 @@ bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size) return NULL; } - size *= nmemb; - - if (size != (unsigned long) size) - { - bfd_set_error (bfd_error_no_memory); - return NULL; - } - - ret = objalloc_alloc ((struct objalloc *) abfd->memory, (unsigned long) size); - if (ret == NULL) - bfd_set_error (bfd_error_no_memory); - return ret; + return bfd_alloc (abfd, size * nmemb); } /* diff --git a/bfd/pe-mips.c b/bfd/pe-mips.c index ec7afc4..940c069 100644 --- a/bfd/pe-mips.c +++ b/bfd/pe-mips.c @@ -339,6 +339,8 @@ static reloc_howto_type howto_table[] = FALSE), /* Pcrel_offset. */ }; +#define NUM_HOWTOS ((sizeof (howto_table) / sizeof (howto_table[0])) + /* Turn a howto into a reloc nunmber. */ #define SELECT_RELOC(x, howto) { x.r_type = howto->type; } @@ -379,7 +381,8 @@ static reloc_howto_type howto_table[] = cache_ptr->addend = - (ptr->section->vma + ptr->value); \ else \ cache_ptr->addend = 0; \ - if (ptr && howto_table[reloc.r_type].pc_relative) \ + if (ptr && reloc.r_type < NUM_HOWTOS \ + && howto_table[reloc.r_type].pc_relative) \ cache_ptr->addend += asect->vma; \ } @@ -509,9 +512,7 @@ coff_mips_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED, { unsigned int i; - for (i = 0; - i < sizeof (howto_table) / sizeof (howto_table[0]); - i++) + for (i = 0; i < NUM_HOWTOS; i++) if (howto_table[i].name != NULL && strcasecmp (howto_table[i].name, r_name) == 0) return &howto_table[i]; diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index ea1459b..d1b33fd 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -458,6 +458,7 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd, aouthdr_int->entry = GET_AOUTHDR_ENTRY (abfd, aouthdr_ext->entry); aouthdr_int->text_start = GET_AOUTHDR_TEXT_START (abfd, aouthdr_ext->text_start); + #if !defined(COFF_WITH_pep) && !defined(COFF_WITH_pex64) /* PE32+ does not have data_start member! */ aouthdr_int->data_start = @@ -505,7 +506,7 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd, int idx; /* PR 17512: Corrupt PE binaries can cause seg-faults. */ - if (a->NumberOfRvaAndSizes > 16) + if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES) { (*_bfd_error_handler) (_("%B: aout header specifies an invalid number of data-directory entries: %d"), @@ -529,6 +530,13 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd, else a->DataDirectory[idx].VirtualAddress = 0; } + + while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES) + { + a->DataDirectory[idx].Size = 0; + a->DataDirectory[idx].VirtualAddress = 0; + idx ++; + } } if (aouthdr_int->entry) @@ -772,7 +780,7 @@ _bfd_XXi_swap_aouthdr_out (bfd * abfd, void * in, void * out) { int idx; - for (idx = 0; idx < 16; idx++) + for (idx = 0; idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES; idx++) { H_PUT_32 (abfd, extra->DataDirectory[idx].VirtualAddress, aouthdr_out->DataDirectory[idx][0]); @@ -1391,7 +1399,9 @@ pe_print_idata (bfd * abfd, void * vfile) break; dll = (char *) data + dll_name - adj; - fprintf (file, _("\n\tDLL Name: %s\n"), dll); + /* PR 17512 file: 078-12277-0.004. */ + bfd_size_type maxlen = (char *)(data + datasize) - dll - 1; + fprintf (file, _("\n\tDLL Name: %.*s\n"), (int) maxlen, dll); if (hint_addr != 0) { @@ -1720,7 +1730,9 @@ pe_print_edata (bfd * abfd, void * vfile) edt.base); /* PR 17512: Handle corrupt PE binaries. */ - if (edt.eat_addr + (edt.num_functions * 4) - adj >= datasize) + if (edt.eat_addr + (edt.num_functions * 4) - adj >= datasize + /* PR 17512 file: 140-165018-0.004. */ + || data + edt.eat_addr - adj < data) fprintf (file, _("\tInvalid Export Address Table rva (0x%lx) or entry count (0x%lx)\n"), (long) edt.eat_addr, (long) edt.num_functions); @@ -1736,11 +1748,12 @@ pe_print_edata (bfd * abfd, void * vfile) /* This rva is to a name (forwarding function) in our section. */ /* Should locate a function descriptor. */ fprintf (file, - "\t[%4ld] +base[%4ld] %04lx %s -- %s\n", + "\t[%4ld] +base[%4ld] %04lx %s -- %.*s\n", (long) i, (long) (i + edt.base), (unsigned long) eat_member, _("Forwarder RVA"), + (int)(datasize - (eat_member - adj)), data + eat_member - adj); } else @@ -1761,11 +1774,14 @@ pe_print_edata (bfd * abfd, void * vfile) _("\n[Ordinal/Name Pointer] Table\n")); /* PR 17512: Handle corrupt PE binaries. */ - if (edt.npt_addr + (edt.num_names * 4) - adj >= datasize) + if (edt.npt_addr + (edt.num_names * 4) - adj >= datasize + || (data + edt.npt_addr - adj) < data) fprintf (file, _("\tInvalid Name Pointer Table rva (0x%lx) or entry count (0x%lx)\n"), (long) edt.npt_addr, (long) edt.num_names); - else if (edt.ot_addr + (edt.num_names * 2) - adj >= datasize) + /* PR 17512: file: 140-147171-0.004. */ + else if (edt.ot_addr + (edt.num_names * 2) - adj >= datasize + || data + edt.ot_addr - adj < data) fprintf (file, _("\tInvalid Ordinal Table rva (0x%lx) or entry count (0x%lx)\n"), (long) edt.ot_addr, (long) edt.num_names); @@ -1786,7 +1802,8 @@ pe_print_edata (bfd * abfd, void * vfile) { char * name = (char *) data + name_ptr - adj; - fprintf (file, "\t[%4ld] %s\n", (long) ord, name); + fprintf (file, "\t[%4ld] %.*s\n", (long) ord, + (int)((char *)(data + datasize) - name), name); } } diff --git a/bfd/peicode.h b/bfd/peicode.h index 157879b..c3d13f8 100644 --- a/bfd/peicode.h +++ b/bfd/peicode.h @@ -271,6 +271,7 @@ pe_mkobject (bfd * abfd) /* in_reloc_p is architecture dependent. */ pe->in_reloc_p = in_reloc_p; + memset (& pe->pe_opthdr, 0, sizeof pe->pe_opthdr); return TRUE; } @@ -1313,7 +1314,7 @@ pe_bfd_object_p (bfd * abfd) /* Swap file header, so that we get the location for calling real_object_p. */ - bfd_coff_swap_filehdr_in (abfd, (PTR)&image_hdr, &internal_f); + bfd_coff_swap_filehdr_in (abfd, &image_hdr, &internal_f); if (! bfd_coff_bad_format_hook (abfd, &internal_f) || internal_f.f_opthdr > bfd_coff_aoutsz (abfd)) @@ -1327,16 +1328,21 @@ pe_bfd_object_p (bfd * abfd) if (opt_hdr_size != 0) { - PTR opthdr; + bfd_size_type amt = opt_hdr_size; + void * opthdr; - opthdr = bfd_alloc (abfd, opt_hdr_size); + /* PR 17521 file: 230-131433-0.004. */ + if (amt < sizeof (PEAOUTHDR)) + amt = sizeof (PEAOUTHDR); + + opthdr = bfd_zalloc (abfd, amt); if (opthdr == NULL) return NULL; if (bfd_bread (opthdr, opt_hdr_size, abfd) != (bfd_size_type) opt_hdr_size) return NULL; - bfd_coff_swap_aouthdr_in (abfd, opthdr, (PTR) & internal_a); + bfd_coff_swap_aouthdr_in (abfd, opthdr, & internal_a); } return coff_real_object_p (abfd, internal_f.f_nscns, &internal_f, -- 2.7.4