This is the mail archive of the binutils-cvs@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] PR22047, Heap out of bounds read in parse_comp_unit


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4b04bba2eb6b646e11a2c38c77667875b3db6828

commit 4b04bba2eb6b646e11a2c38c77667875b3db6828
Author: Alan Modra <amodra@gmail.com>
Date:   Sun Oct 1 12:07:59 2017 +1030

    PR22047, Heap out of bounds read in parse_comp_unit
    
    Like the PR22230 fix, we can allocate a buffer with an extra byte
    rather than letting bfd_simple_get_relocated_section_contents malloc
    and return a buffer.  Much better than allocating another buffer
    afterwards.
    
    	PR 22047
    	* dwarf2.c (read_section): Allocate buffer with extra byte for
    	bfd_simple_get_relocated_section_contents rather than copying
    	afterwards.

Diff:
---
 bfd/ChangeLog |  7 +++++++
 bfd/dwarf2.c  | 53 +++++++++++++++--------------------------------------
 2 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8b44ceb..9a6af67 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2017-10-01  Alan Modra  <amodra@gmail.com>
+
+	PR 22047
+	* dwarf2.c (read_section): Allocate buffer with extra byte for
+	bfd_simple_get_relocated_section_contents rather than copying
+	afterwards.
+
 2017-09-29  Alan Modra  <amodra@gmail.com>
 
 	* merge.c (merge_strings): Return FALSE on malloc failure.
diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index be415e3..4cf50cc 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -526,9 +526,10 @@ read_section (bfd *           abfd,
 {
   asection *msec;
   const char *section_name = sec->uncompressed_name;
+  bfd_byte *contents = *section_buffer;
 
   /* The section may have already been read.  */
-  if (*section_buffer == NULL)
+  if (contents == NULL)
     {
       msec = bfd_get_section_by_name (abfd, section_name);
       if (! msec)
@@ -546,45 +547,21 @@ read_section (bfd *           abfd,
 	}
 
       *section_size = msec->rawsize ? msec->rawsize : msec->size;
-      if (syms)
-	{
-	  *section_buffer
-	    = bfd_simple_get_relocated_section_contents (abfd, msec, NULL, syms);
-	  if (! *section_buffer)
-	    return FALSE;
-	}
-      else
-	{
-	  *section_buffer = (bfd_byte *) bfd_malloc (*section_size);
-	  if (! *section_buffer)
-	    return FALSE;
-	  if (! bfd_get_section_contents (abfd, msec, *section_buffer,
-					  0, *section_size))
-	    return FALSE;
-	}
-
-      /* Paranoia - if we are reading in a string section, make sure that it
-	 is NUL terminated.  This is to prevent string functions from running
-	 off the end of the buffer.  Note - knowing the size of the buffer is
-	 not enough as some functions, eg strchr, do not have a range limited
-	 equivalent.
-
-	 FIXME: We ought to use a flag in the dwarf_debug_sections[] table to
-	 determine the nature of a debug section, rather than checking the
-	 section name as we do here.  */
-      if (*section_size > 0
-	  && (*section_buffer)[*section_size - 1] != 0
-	  && (strstr (section_name, "_str") || strstr (section_name, "names")))
+      /* Paranoia - alloc one extra so that we can make sure a string
+	 section is NUL terminated.  */
+      contents = (bfd_byte *) bfd_malloc (*section_size + 1);
+      if (contents == NULL)
+	return FALSE;
+      if (syms
+	  ? !bfd_simple_get_relocated_section_contents (abfd, msec, contents,
+							syms)
+	  : !bfd_get_section_contents (abfd, msec, contents, 0, *section_size))
 	{
-	  bfd_byte * new_buffer = malloc (*section_size + 1);
-
-	  _bfd_error_handler (_("warning: dwarf string section '%s' is not NUL terminated"),
-			      section_name);
-	  memcpy (new_buffer, *section_buffer, *section_size);
-	  new_buffer[*section_size] = 0;
-	  free (*section_buffer);
-	  *section_buffer = new_buffer;
+	  free (contents);
+	  return FALSE;
 	}
+      contents[*section_size] = 0;
+      *section_buffer = contents;
     }
 
   /* It is possible to get a bad value for the offset into the section


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]