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.

2 files changed, 80 insertions, 20 deletions

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 131925a..709c1cc 100644
--- 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 71fc827..c00b62b 100644
--- 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)
 
-/* 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"));