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.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 5c2445c63bd644c5f71ff923db0aca4136235b89..4b4086041924c87d4bd179af68917fc79359ec66 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,21 @@
+2014-11-05  Nick Clifton  <nickc@redhat.com>
+
+       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  <nickc@redhat.com>
 
        PR binutils/17512

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 48709f4dec799ab2d9454b5627c11dfc1080b490..ab76083744785a41b13a9d2b86bc0b219e250b23 100644 (file)
--- 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 f1a1189f8a8fb5d4e3cf5686559564aacafac690..d0314301f673d855bcd2d47d40ead691866101a2 100644 (file)
--- 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  <none>",
                         member_high, member,
                         WithoutHighBit (member_high), member);
+             /* PR binutils/17512: Handle corrupt PE data.  */
+             else if (member - adj + 2 >= datasize)
+               fprintf (file, _("\t<corrupt: 0x%04lx>"), member);
              else
                {
                  int ordinal;
@@ -1498,6 +1501,9 @@ pe_print_idata (bfd * abfd, void * vfile)
              if (HighBitSet (member))
                fprintf (file, "\t%04lx\t %4lu  <none>",
                         member, WithoutHighBit (member));
+             /* PR binutils/17512: Handle corrupt PE data.  */
+             else if (member - adj + 2 >= datasize)
+               fprintf (file, _("\t<corrupt: 0x%04lx>"), 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] <corrupt offset: %lx>\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, _("<corrupt string length: %#x>"), len);
+           {
+             fprintf (file, _("<corrupt string length: %#x>\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, _("<corrupt string offset: %#lx>"), entry);
+       {
+         fprintf (file, _("<corrupt string offset: %#lx>\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, "<unknown>"); break;
+    default:
+      fprintf (file, _("<unknown directory type: %d>\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;