More fixes for problems exposed by valgrind and the address sanitizer
authorNick Clifton <nickc@redhat.com>
Mon, 10 Nov 2014 14:18:45 +0000 (14:18 +0000)
committerNick Clifton <nickc@redhat.com>
Mon, 10 Nov 2014 14:27:38 +0000 (14:27 +0000)
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
bfd/coff-i386.c
bfd/coff-x86_64.c
bfd/coffcode.h
bfd/coffgen.c
bfd/opncls.c
bfd/pe-mips.c
bfd/peXXigen.c
bfd/peicode.h

index 23c8419..ab34855 100644 (file)
@@ -1,3 +1,40 @@
+2014-11-10  Nick Clifton  <nickc@redhat.com>
+
+       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  <amodra@gmail.com>
 
        * peXXigen.c (pe_print_idata): Revert last patch, cast lhs instead.
index 87b014b..848d69b 100644 (file)
@@ -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];
index 8d23733..2a21bb8 100644 (file)
@@ -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];
index 3abb6a3..8c6b1dd 100644 (file)
@@ -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;
index 9ad0783..6c69902 100644 (file)
@@ -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) _("<corrupt>");
              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) _("<corrupt>");
                  else
                    internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t)
index a2a35f4..a22fba0 100644 (file)
@@ -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);
 }
 
 /*
index ec7afc4..940c069 100644 (file)
@@ -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];
index ea1459b..d1b33fd 100644 (file)
@@ -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);
        }
     }
 
index 157879b..c3d13f8 100644 (file)
@@ -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,