From 20ad5e2842911039a60b6bdf9880cee895179e43 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Wed, 5 Nov 2014 10:13:16 +0000 Subject: [PATCH] More fixes for processing corrupt files. PR binutils/17512 * coffcode.h (coff_set_alignment_hook): Warn if the file lies about the number of relocations it contains. (coff_sort_func_alent): Return 0 if the pointers are NULL. (coff_slurp_line_table): Add more range checks. Do not free new tables created when sorting line numbers. * peXXigen.c (pe_print_idata): Add range checks. (pe_print_edata): Likewise. (rsrc_print_resource_entries): Likewise. Avoid printing control characters. Terminate priniting if corruption is detected. (rsrc_print_resource_directory): Terminate printing if an unknown directory type is encountered. (pe_print_debugdata): Fix off-by-one error. (rsrc_count_entries): Add range checking. (rsrc_parse_entry): Likewise. --- bfd/ChangeLog | 18 +++++++++ bfd/coffcode.h | 28 ++++++++++--- bfd/peXXigen.c | 122 +++++++++++++++++++++++++++++++++++++++++---------------- 3 files changed, 129 insertions(+), 39 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 5c2445c..4b40860 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,21 @@ +2014-11-05 Nick Clifton + + PR binutils/17512 + * coffcode.h (coff_set_alignment_hook): Warn if the file lies + about the number of relocations it contains. + (coff_sort_func_alent): Return 0 if the pointers are NULL. + (coff_slurp_line_table): Add more range checks. Do not free new + tables created when sorting line numbers. + * peXXigen.c (pe_print_idata): Add range checks. + (pe_print_edata): Likewise. + (rsrc_print_resource_entries): Likewise. Avoid printing control + characters. Terminate priniting if corruption is detected. + (rsrc_print_resource_directory): Terminate printing if an unknown + directory type is encountered. + (pe_print_debugdata): Fix off-by-one error. + (rsrc_count_entries): Add range checking. + (rsrc_parse_entry): Likewise. + 2014-11-04 Nick Clifton PR binutils/17512 diff --git a/bfd/coffcode.h b/bfd/coffcode.h index 48709f4..ab76083 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -1920,6 +1920,15 @@ coff_set_alignment_hook (bfd * abfd ATTRIBUTE_UNUSED, if (bfd_seek (abfd, oldpos, 0) != 0) return; section->reloc_count = hdr->s_nreloc = n.r_vaddr - 1; + /* PR binutils/17512: Stop corrupt files from causing + memory problems if they claim to have too many relocs. */ + if (section->reloc_count * relsz > (bfd_size_type) bfd_get_size (abfd)) + { + (*_bfd_error_handler) + ("%s: warning: claims to have %#x relocs, but the file is not that big", + bfd_get_filename (abfd), section->reloc_count); + section->reloc_count = 0; + } section->rel_filepos += relsz; } else if (hdr->s_nreloc == 0xffff) @@ -4494,6 +4503,8 @@ coff_sort_func_alent (const void * arg1, const void * arg2) const coff_symbol_type *s1 = (const coff_symbol_type *) (al1->u.sym); const coff_symbol_type *s2 = (const coff_symbol_type *) (al2->u.sym); + if (s1 == NULL || s2 == NULL) + return 0; if (s1->symbol.value < s2->symbol.value) return -1; else if (s1->symbol.value > s2->symbol.value) @@ -4518,7 +4529,9 @@ coff_slurp_line_table (bfd *abfd, asection *asect) BFD_ASSERT (asect->lineno == NULL); amt = ((bfd_size_type) asect->lineno_count + 1) * sizeof (alent); - lineno_cache = (alent *) bfd_alloc (abfd, amt); + if (amt > (bfd_size_type) bfd_get_size (abfd)) + return FALSE; + lineno_cache = (alent *) bfd_zalloc (abfd, amt); if (lineno_cache == NULL) return FALSE; @@ -4566,6 +4579,9 @@ coff_slurp_line_table (bfd *abfd, asection *asect) ((symndx + obj_raw_syments (abfd)) ->u.syment._n._n_n._n_zeroes)); cache_ptr->u.sym = (asymbol *) sym; + if (sym == NULL) + continue; + if (sym->lineno != NULL) (*_bfd_error_handler) (_("%B: warning: duplicate line number information for `%s'"), @@ -4609,7 +4625,7 @@ coff_slurp_line_table (bfd *abfd, asection *asect) /* Create the new sorted table. */ amt = ((bfd_size_type) asect->lineno_count + 1) * sizeof (alent); - n_lineno_cache = (alent *) bfd_alloc (abfd, amt); + n_lineno_cache = (alent *) bfd_zalloc (abfd, amt); if (n_lineno_cache != NULL) { alent *n_cache_ptr = n_lineno_cache; @@ -4621,8 +4637,9 @@ coff_slurp_line_table (bfd *abfd, asection *asect) /* Copy the function entry and update it. */ *n_cache_ptr = *old_ptr; - sym = (coff_symbol_type *)n_cache_ptr->u.sym; - sym->lineno = n_cache_ptr; + sym = (coff_symbol_type *) n_cache_ptr->u.sym; + if (sym != NULL) + sym->lineno = n_cache_ptr; n_cache_ptr++; old_ptr++; @@ -4633,7 +4650,8 @@ coff_slurp_line_table (bfd *abfd, asection *asect) n_cache_ptr->line_number = 0; memcpy (lineno_cache, n_lineno_cache, amt); } - bfd_release (abfd, func_table); + /* PR binutils/17512: Do *not* free the func table + and new lineno cache - they are now being used. */ } } diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index f1a1189..d031430 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -1466,6 +1466,9 @@ pe_print_idata (bfd * abfd, void * vfile) fprintf (file, "\t%lx%08lx\t %4lx%08lx ", member_high, member, WithoutHighBit (member_high), member); + /* PR binutils/17512: Handle corrupt PE data. */ + else if (member - adj + 2 >= datasize) + fprintf (file, _("\t"), member); else { int ordinal; @@ -1498,6 +1501,9 @@ pe_print_idata (bfd * abfd, void * vfile) if (HighBitSet (member)) fprintf (file, "\t%04lx\t %4lu ", member, WithoutHighBit (member)); + /* PR binutils/17512: Handle corrupt PE data. */ + else if (member - adj + 2 >= datasize) + fprintf (file, _("\t"), member); else { int ordinal; @@ -1765,19 +1771,23 @@ pe_print_edata (bfd * abfd, void * vfile) (long) edt.num_names); else for (i = 0; i < edt.num_names; ++i) { - bfd_vma name_ptr = bfd_get_32 (abfd, - data + - edt.npt_addr - + (i*4) - adj); + bfd_vma name_ptr; + bfd_vma ord; - char *name = (char *) data + name_ptr - adj; + ord = bfd_get_16 (abfd, data + edt.ot_addr + (i * 2) - adj); + name_ptr = bfd_get_32 (abfd, data + edt.npt_addr + (i * 4) - adj); - bfd_vma ord = bfd_get_16 (abfd, - data + - edt.ot_addr - + (i*2) - adj); - fprintf (file, - "\t[%4ld] %s\n", (long) ord, name); + if ((name_ptr - adj) >= datasize) + { + fprintf (file, _("\t[%4ld] \n"), + (long) ord, (long) name_ptr); + } + else + { + char * name = (char *) data + name_ptr - adj; + + fprintf (file, "\t[%4ld] %s\n", (long) ord, name); + } } free (data); @@ -2217,6 +2227,12 @@ static bfd_byte * rsrc_print_resource_directory (FILE * , bfd *, unsigned int, bfd_byte *, rsrc_regions *, bfd_vma); +/* Print the resource entry at DATA, with the text indented by INDENT. + Recusively calls rsrc_print_resource_directory to print the contents + of directory entries. + Returns the address of the end of the data associated with the entry + or section_end + 1 upon failure. */ + static bfd_byte * rsrc_print_resource_entries (FILE * file, bfd * abfd, @@ -2233,7 +2249,7 @@ rsrc_print_resource_entries (FILE * file, fprintf (file, _("%03x %*.s Entry: "), (int)(data - regions->section_start), indent, " "); - entry = (long) bfd_get_32 (abfd, data); + entry = (unsigned long) bfd_get_32 (abfd, data); if (is_name) { bfd_byte * name; @@ -2246,7 +2262,7 @@ rsrc_print_resource_entries (FILE * file, else name = regions->section_start + entry - rva_bias; - if (name + 2 < regions->section_end) + if (name + 2 < regions->section_end && name > regions->section_start) { unsigned int len; @@ -2256,20 +2272,38 @@ rsrc_print_resource_entries (FILE * file, len = bfd_get_16 (abfd, name); fprintf (file, _("name: [val: %08lx len %d]: "), entry, len); + if (name + 2 + len * 2 < regions->section_end) { /* This strange loop is to cope with multibyte characters. */ while (len --) { + char c; + name += 2; - fprintf (file, "%.1s", name); + c = * name; + /* Avoid printing control characters. */ + if (c > 0 && c < 32) + fprintf (file, "^%c", c + 64); + else + fprintf (file, "%.1s", name); } } else - fprintf (file, _(""), len); + { + fprintf (file, _("\n"), len); + /* PR binutils/17512: Do not try to continue decoding a + corrupted resource section. It is likely to end up with + reams of extraneous output. FIXME: We could probably + continue if we disable the printing of strings... */ + return regions->section_end + 1; + } } else - fprintf (file, _(""), entry); + { + fprintf (file, _("\n"), entry); + return regions->section_end + 1; + } } else fprintf (file, _("ID: %#08lx"), entry); @@ -2278,9 +2312,16 @@ rsrc_print_resource_entries (FILE * file, fprintf (file, _(", Value: %#08lx\n"), entry); if (HighBitSet (entry)) - return rsrc_print_resource_directory (file, abfd, indent + 1, - regions->section_start + WithoutHighBit (entry), - regions, rva_bias); + { + data = regions->section_start + WithoutHighBit (entry); + if (data <= regions->section_start || data > regions->section_end) + return regions->section_end + 1; + + /* FIXME: PR binutils/17512: A corrupt file could contain a loop + in the resource table. We need some way to detect this. */ + return rsrc_print_resource_directory (file, abfd, indent + 1, data, + regions, rva_bias); + } if (regions->section_start + entry + 16 >= regions->section_end) return regions->section_end + 1; @@ -2327,7 +2368,12 @@ rsrc_print_resource_directory (FILE * file, case 0: fprintf (file, "Type"); break; case 2: fprintf (file, "Name"); break; case 4: fprintf (file, "Language"); break; - default: fprintf (file, ""); break; + default: + fprintf (file, _("\n"), indent); + /* FIXME: For now we end the printing here. If in the + future more directory types are added to the RSRC spec + then we will need to change this. */ + return regions->section_end + 1; } fprintf (file, _(" Table: Char: %d, Time: %08lx, Ver: %d/%d, Num Names: %d, IDs: %d\n"), @@ -2449,10 +2495,10 @@ rsrc_print_section (bfd * abfd, void * vfile) } if (regions.strings_start != NULL) - fprintf (file, " String table starts at %03x\n", + fprintf (file, " String table starts at offset: %#03x\n", (int) (regions.strings_start - regions.section_start)); if (regions.resource_start != NULL) - fprintf (file, " Resources start at %03xx\n", + fprintf (file, " Resources start at offset: %#03x\n", (int) (regions.resource_start - regions.section_start)); free (regions.section_start); @@ -2547,7 +2593,7 @@ pe_print_debugdata (bfd * abfd, void * vfile) _bfd_XXi_swap_debugdir_in (abfd, ext, &idd); - if ((idd.Type) > IMAGE_NUMBEROF_DEBUG_TYPES) + if ((idd.Type) >= IMAGE_NUMBEROF_DEBUG_TYPES) type_name = debug_type_names[0]; else type_name = debug_type_names[idd.Type]; @@ -2953,7 +2999,7 @@ rsrc_count_entries (bfd * abfd, else name = datastart + entry - rva_bias; - if (name + 2 >= dataend) + if (name + 2 >= dataend || name < datastart) return dataend + 1; unsigned int len = bfd_get_16 (abfd, name); @@ -2964,10 +3010,14 @@ rsrc_count_entries (bfd * abfd, entry = (long) bfd_get_32 (abfd, data + 4); if (HighBitSet (entry)) - return rsrc_count_directory (abfd, - datastart, - datastart + WithoutHighBit (entry), - dataend, rva_bias); + { + data = datastart + WithoutHighBit (entry); + + if (data <= datastart || data >= dataend) + return dataend + 1; + + return rsrc_count_directory (abfd, datastart, data, dataend, rva_bias); + } if (datastart + entry + 16 >= dataend) return dataend + 1; @@ -3089,20 +3139,24 @@ rsrc_parse_entry (bfd * abfd, if (is_name) { - /* FIXME: Add range checking ? */ + bfd_byte * address; + if (HighBitSet (val)) { val = WithoutHighBit (val); - entry->name_id.name.len = bfd_get_16 (abfd, datastart + val); - entry->name_id.name.string = datastart + val + 2; + address = datastart + val; } else { - entry->name_id.name.len = bfd_get_16 (abfd, datastart + val - - rva_bias); - entry->name_id.name.string = datastart + val - rva_bias + 2; + address = datastart + val - rva_bias; } + + if (address + 3 > dataend) + return dataend; + + entry->name_id.name.len = bfd_get_16 (abfd, address); + entry->name_id.name.string = address + 2; } else entry->name_id.id = val; -- 2.7.4