Fixes an infinite loop in readelf parsing a corrupt binary, and other minor corrections.

	PR binutils/17531
	* readelf.c (get_data): Move excessive length check to earlier on
	in the function and allow for wraparound in the arithmetic.
	(get_32bit_elf_symbols): Terminate early if the section size is
	zero.  Check for an invalid sh_entsize.  Check for an index
	section with an invalid size.
	(get_64bit_elf_symbols): Likewise.
	(process_section_groups): Check for an invalid sh_entsize.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 131925a31920f601488cef28d5a8a2fe214d474c..709c1cc6055c6f71f4a53720d5bb8b2ec53f2e23 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,15 @@
+2014-11-27  Espen Grindhaug <espen@grindhaug.org>
+           Nick Clifton  <nickc@redhat.com>
+
+       PR binutils/17531
+       * readelf.c (get_data): Move excessive length check to earlier on
+       in the function and allow for wraparound in the arithmetic.
+       (get_32bit_elf_symbols): Terminate early if the section size is
+       zero.  Check for an invalid sh_entsize.  Check for an index
+       section with an invalid size.
+       (get_64bit_elf_symbols): Likewise.
+       (process_section_groups): Check for an invalid sh_entsize.
+
 2014-11-24  Mark Wielaard  <mjw@redhat.com>
 
        * dwarf.c (read_and_display_attr_value): Handle DW_LANG_C11,

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 71fc827f28eef535fb382131729c4fd65c6b3a8c..c00b62b908eea3feaae41cf542c256b942786db9 100644 (file)
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -165,7 +165,7 @@
 #endif
 
 char * program_name = "readelf";
-static long archive_file_offset;
+static unsigned long archive_file_offset;
 static unsigned long archive_file_size;
 static bfd_size_type current_file_size;
 static unsigned long dynamic_addr;
@@ -313,36 +313,40 @@ static const char *get_symbol_version_string
     }                                          \
   while (0)
 \f
-/* Retrieve NMEMB structures, each SIZE bytes long from FILE starting at OFFSET.
+/* Retrieve NMEMB structures, each SIZE bytes long from FILE starting at OFFSET +
+   the offset of the current archive member, if we are examining an archive.
    Put the retrieved data into VAR, if it is not NULL.  Otherwise allocate a buffer
    using malloc and fill that.  In either case return the pointer to the start of
    the retrieved data or NULL if something went wrong.  If something does go wrong
-   emit an error message using REASON as part of the context.  */
+   and REASON is not NULL then emit an error message using REASON as part of the
+   context.  */
 
 static void *
-get_data (void * var, FILE * file, long offset, size_t size, size_t nmemb,
+get_data (void * var, FILE * file, unsigned long offset, size_t size, size_t nmemb,
          const char * reason)
 {
   void * mvar;
+  size_t amt = size * nmemb;
 
   if (size == 0 || nmemb == 0)
     return NULL;
 
-  if (fseek (file, archive_file_offset + offset, SEEK_SET))
+  /* Be kind to memory chekers (eg valgrind, address sanitizer) by not
+     attempting to allocate memory when the read is bound to fail.  */
+  if (amt > current_file_size
+      || offset + archive_file_offset + amt > current_file_size)
     {
       if (reason)
-       error (_("Unable to seek to 0x%lx for %s\n"),
-              (unsigned long) archive_file_offset + offset, reason);
+       error (_("Reading 0x%lx bytes extends past end of file for %s\n"),
+              (unsigned long) amt, reason);
       return NULL;
     }
 
-  /* Be kind to memory chekers (eg valgrind, address sanitizer) by not
-     attempting to allocate memory when the read is bound to fail.  */
-  if (offset + archive_file_offset + size * nmemb > current_file_size)
+  if (fseek (file, archive_file_offset + offset, SEEK_SET))
     {
       if (reason)
-       error (_("Reading 0x%lx bytes extends past end of file for %s\n"),
-              (unsigned long) (size * nmemb), reason);
+       error (_("Unable to seek to 0x%lx for %s\n"),
+              (unsigned long) archive_file_offset + offset, reason);
       return NULL;
     }
 
@@ -362,14 +366,14 @@ get_data (void * var, FILE * file, long offset, size_t size, size_t nmemb,
          return NULL;
        }
 
-      ((char *) mvar)[size * nmemb] = '\0';
+      ((char *) mvar)[amt] = '\0';
     }
 
   if (fread (mvar, size, nmemb, file) != nmemb)
     {
       if (reason)
        error (_("Unable to read in 0x%lx bytes of %s\n"),
-              (unsigned long)(size * nmemb), reason);
+              (unsigned long) amt, reason);
       if (mvar != var)
        free (mvar);
       return NULL;
@@ -4756,10 +4760,18 @@ get_32bit_elf_symbols (FILE * file,
   Elf_Internal_Sym * psym;
   unsigned int j;
 
+  if (section->sh_size == 0)
+    {
+      if (num_syms_return != NULL)
+       * num_syms_return = 0;
+      return NULL;
+    }
+
   /* Run some sanity checks first.  */
-  if (section->sh_entsize == 0)
+  if (section->sh_entsize == 0 || section->sh_entsize > section->sh_size)
     {
-      error (_("sh_entsize is zero\n"));
+      error (_("Section %s has an invalid sh_entsize of 0x%lx\n"),
+            printable_section_name (section), (unsigned long) section->sh_entsize);
       goto exit_point;
     }
 
@@ -4774,7 +4786,8 @@ get_32bit_elf_symbols (FILE * file,
 
   if (number * sizeof (Elf32_External_Sym) > section->sh_size + 1)
     {
-      error (_("Invalid sh_entsize\n"));
+      error (_("Size (0x%lx) of section %s is not a multiple of its sh_entsize (0x%lx)\n"),
+            section->sh_size, printable_section_name (section), section->sh_entsize);
       goto exit_point;
     }
 
@@ -4794,6 +4807,15 @@ get_32bit_elf_symbols (FILE * file,
                                                    _("symbol table section indicies"));
       if (shndx == NULL)
        goto exit_point;
+      /* PR17531: file: heap-buffer-overflow */
+      else if (symtab_shndx_hdr->sh_size / sizeof (Elf_External_Sym_Shndx) < number)
+       {
+         error (_("Index section %s has an sh_size of 0x%lx - expected 0x%lx\n"),
+                printable_section_name (symtab_shndx_hdr),
+                (unsigned long) symtab_shndx_hdr->sh_size,
+                (unsigned long) section->sh_size);
+         goto exit_point;
+       }
     }
 
   isyms = (Elf_Internal_Sym *) cmalloc (number, sizeof (Elf_Internal_Sym));
@@ -4844,10 +4866,18 @@ get_64bit_elf_symbols (FILE * file,
   Elf_Internal_Sym * psym;
   unsigned int j;
 
+  if (section->sh_size == 0)
+    {
+      if (num_syms_return != NULL)
+       * num_syms_return = 0;
+      return NULL;
+    }
+
   /* Run some sanity checks first.  */
-  if (section->sh_entsize == 0)
+  if (section->sh_entsize == 0 || section->sh_entsize > section->sh_size)
     {
-      error (_("sh_entsize is zero\n"));
+      error (_("Section %s has an invalid sh_entsize of 0x%lx\n"),
+            printable_section_name (section), (unsigned long) section->sh_entsize);
       goto exit_point;
     }
 
@@ -4862,7 +4892,8 @@ get_64bit_elf_symbols (FILE * file,
 
   if (number * sizeof (Elf64_External_Sym) > section->sh_size + 1)
     {
-      error (_("Invalid sh_entsize\n"));
+      error (_("Size (0x%lx) of section %s is not a multiple of its sh_entsize (0x%lx)\n"),
+            section->sh_size, printable_section_name (section), section->sh_entsize);
       goto exit_point;
     }
 
@@ -4881,6 +4912,14 @@ get_64bit_elf_symbols (FILE * file,
                                                    _("symbol table section indicies"));
       if (shndx == NULL)
        goto exit_point;
+      else if (symtab_shndx_hdr->sh_size / sizeof (Elf_External_Sym_Shndx) < number)
+       {
+         error (_("Index section %s has an sh_size of 0x%lx - expected 0x%lx\n"),
+                printable_section_name (symtab_shndx_hdr),
+                (unsigned long) symtab_shndx_hdr->sh_size,
+                (unsigned long) section->sh_size);
+         goto exit_point;
+       }
     }
 
   isyms = (Elf_Internal_Sym *) cmalloc (number, sizeof (Elf_Internal_Sym));
@@ -5796,6 +5835,15 @@ process_section_groups (FILE * file)
                ? strtab + sym->st_name : _("<corrupt>");
            }
 
+         /* PR 17531: file: loop.  */
+         if (section->sh_entsize > section->sh_size)
+           {
+             error (_("Section %s has sh_entsize (0x%lx) which is larger than its size (0x%lx)\n"),
+                    printable_section_name (section),
+                    section->sh_entsize, section->sh_size);
+             break;
+           }
+
          start = (unsigned char *) get_data (NULL, file, section->sh_offset,
                                               1, section->sh_size,
                                               _("section data"));