Re: BFD overflows

[Alan Modra <amodra@bigpond.net.au>]

On Sat, May 07, 2005 at 09:14:41PM -0400, Mike Frysinger wrote:
> however, at least one issue still remains.  find attached a small binary 
> which, when you run `strings` on it, triggers a segfault:

	* elfcode.h (elf_object_p): Add more sanity checks on elf header.

Applying mainline.

Index: bfd/elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.67
diff -u -p -r1.67 elfcode.h
--- bfd/elfcode.h	4 May 2005 15:53:28 -0000	1.67
+++ bfd/elfcode.h	8 May 2005 11:18:23 -0000
@@ -612,8 +612,13 @@ elf_object_p (bfd *abfd)
 
   if (i_ehdrp->e_shoff != 0)
     {
+      bfd_signed_vma where = i_ehdrp->e_shoff;
+
+      if (where != (file_ptr) where)
+	goto got_wrong_format_error;
+
       /* Seek to the section header table in the file.  */
-      if (bfd_seek (abfd, (file_ptr) i_ehdrp->e_shoff, SEEK_SET) != 0)
+      if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
 	goto got_no_match;
 
       /* Read the first section header at index 0, and convert to internal
@@ -625,13 +630,50 @@ elf_object_p (bfd *abfd)
       /* If the section count is zero, the actual count is in the first
 	 section header.  */
       if (i_ehdrp->e_shnum == SHN_UNDEF)
-	i_ehdrp->e_shnum = i_shdr.sh_size;
+	{
+	  i_ehdrp->e_shnum = i_shdr.sh_size;
+	  if (i_ehdrp->e_shnum != i_shdr.sh_size)
+	    goto got_wrong_format_error;
+	}
 
       /* And similarly for the string table index.  */
       if (i_ehdrp->e_shstrndx == SHN_XINDEX)
-	i_ehdrp->e_shstrndx = i_shdr.sh_link;
+	{
+	  i_ehdrp->e_shstrndx = i_shdr.sh_link;
+	  if (i_ehdrp->e_shstrndx != i_shdr.sh_link)
+	    goto got_wrong_format_error;
+	}
+
+      /* Sanity check that we can read all of the section headers.
+	 It ought to be good enough to just read the last one.  */
+      if (i_ehdrp->e_shnum != 1)
+	{
+	  /* Check that we don't have a totally silly number of sections.  */
+	  if (i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (x_shdr))
+	    goto got_wrong_format_error;
+
+	  where += (i_ehdrp->e_shnum - 1) * sizeof (x_shdr);
+	  if (where != (file_ptr) where)
+	    goto got_wrong_format_error;
+	  if ((bfd_size_type) where <= i_ehdrp->e_shoff)
+	    goto got_wrong_format_error;
+
+	  if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+	    goto got_no_match;
+	  if (bfd_bread (&x_shdr, sizeof x_shdr, abfd) != sizeof (x_shdr))
+	    goto got_no_match;
+
+	  /* Back to where we were.  */
+	  where = i_ehdrp->e_shoff + sizeof (x_shdr);
+	  if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+	    goto got_no_match;
+	}
     }
 
+  /* A further sanity check.  */
+  if (i_ehdrp->e_shstrndx >= i_ehdrp->e_shnum)
+    goto got_wrong_format_error;
+
   /* Allocate space for a copy of the section header table in
      internal form.  */
   if (i_ehdrp->e_shnum != 0)

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre