[binutils-gdb] PR22202, buffer overflow in parse_die

[Alan Modra]

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

commit 1da5c9a485f3dcac4c45e96ef4b7dae5948314b5
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Sep 25 20:20:38 2017 +0930

    PR22202, buffer overflow in parse_die
    
    There was a complete lack of sanity checking in dwarf1.c
    
    	PR 22202
    	* dwarf1.c (parse_die): Sanity check pointer against section limit
    	before dereferencing.
    	(parse_line_table): Likewise.

Diff:
---
 bfd/ChangeLog |  7 +++++++
 bfd/dwarf1.c  | 56 ++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index ee286c4..54199ad 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,5 +1,12 @@
 2017-09-25  Alan Modra  <amodra@gmail.com>
 
+	PR 22202
+	* dwarf1.c (parse_die): Sanity check pointer against section limit
+	before dereferencing.
+	(parse_line_table): Likewise.
+
+2017-09-25  Alan Modra  <amodra@gmail.com>
+
 	PR 22201
 	* dwarf2.c (scan_unit_for_symbols): Ignore DW_AT_name unless it
 	has string form.
diff --git a/bfd/dwarf1.c b/bfd/dwarf1.c
index 37d0e82..2d641a7 100644
--- a/bfd/dwarf1.c
+++ b/bfd/dwarf1.c
@@ -189,11 +189,14 @@ parse_die (bfd *             abfd,
   memset (aDieInfo, 0, sizeof (* aDieInfo));
 
   /* First comes the length.  */
-  aDieInfo->length = bfd_get_32 (abfd, (bfd_byte *) xptr);
+  if (xptr + 4 > aDiePtrEnd)
+    return FALSE;
+  aDieInfo->length = bfd_get_32 (abfd, xptr);
   xptr += 4;
   if (aDieInfo->length == 0
-      || (this_die + aDieInfo->length) >= aDiePtrEnd)
+      || this_die + aDieInfo->length > aDiePtrEnd)
     return FALSE;
+  aDiePtrEnd = this_die + aDieInfo->length;
   if (aDieInfo->length < 6)
     {
       /* Just padding bytes.  */
@@ -202,18 +205,20 @@ parse_die (bfd *             abfd,
     }
 
   /* Then the tag.  */
-  aDieInfo->tag = bfd_get_16 (abfd, (bfd_byte *) xptr);
+  if (xptr + 2 > aDiePtrEnd)
+    return FALSE;
+  aDieInfo->tag = bfd_get_16 (abfd, xptr);
   xptr += 2;
 
   /* Then the attributes.  */
-  while (xptr < (this_die + aDieInfo->length))
+  while (xptr + 2 <= aDiePtrEnd)
     {
       unsigned short attr;
 
       /* Parse the attribute based on its form.  This section
          must handle all dwarf1 forms, but need only handle the
 	 actual attributes that we care about.  */
-      attr = bfd_get_16 (abfd, (bfd_byte *) xptr);
+      attr = bfd_get_16 (abfd, xptr);
       xptr += 2;
 
       switch (FORM_FROM_ATTR (attr))
@@ -223,12 +228,15 @@ parse_die (bfd *             abfd,
 	  break;
 	case FORM_DATA4:
 	case FORM_REF:
-	  if (attr == AT_sibling)
-	    aDieInfo->sibling = bfd_get_32 (abfd, (bfd_byte *) xptr);
-	  else if (attr == AT_stmt_list)
+	  if (xptr + 4 <= aDiePtrEnd)
 	    {
-	      aDieInfo->stmt_list_offset = bfd_get_32 (abfd, (bfd_byte *) xptr);
-	      aDieInfo->has_stmt_list = 1;
+	      if (attr == AT_sibling)
+		aDieInfo->sibling = bfd_get_32 (abfd, xptr);
+	      else if (attr == AT_stmt_list)
+		{
+		  aDieInfo->stmt_list_offset = bfd_get_32 (abfd, xptr);
+		  aDieInfo->has_stmt_list = 1;
+		}
 	    }
 	  xptr += 4;
 	  break;
@@ -236,22 +244,29 @@ parse_die (bfd *             abfd,
 	  xptr += 8;
 	  break;
 	case FORM_ADDR:
-	  if (attr == AT_low_pc)
-	    aDieInfo->low_pc = bfd_get_32 (abfd, (bfd_byte *) xptr);
-	  else if (attr == AT_high_pc)
-	    aDieInfo->high_pc = bfd_get_32 (abfd, (bfd_byte *) xptr);
+	  if (xptr + 4 <= aDiePtrEnd)
+	    {
+	      if (attr == AT_low_pc)
+		aDieInfo->low_pc = bfd_get_32 (abfd, xptr);
+	      else if (attr == AT_high_pc)
+		aDieInfo->high_pc = bfd_get_32 (abfd, xptr);
+	    }
 	  xptr += 4;
 	  break;
 	case FORM_BLOCK2:
-	  xptr += 2 + bfd_get_16 (abfd, (bfd_byte *) xptr);
+	  if (xptr + 2 <= aDiePtrEnd)
+	    xptr += bfd_get_16 (abfd, xptr);
+	  xptr += 2;
 	  break;
 	case FORM_BLOCK4:
-	  xptr += 4 + bfd_get_32 (abfd, (bfd_byte *) xptr);
+	  if (xptr + 4 <= aDiePtrEnd)
+	    xptr += bfd_get_32 (abfd, xptr);
+	  xptr += 4;
 	  break;
 	case FORM_STRING:
 	  if (attr == AT_name)
 	    aDieInfo->name = (char *) xptr;
-	  xptr += strlen ((char *) xptr) + 1;
+	  xptr += strnlen ((char *) xptr, aDiePtrEnd - xptr) + 1;
 	  break;
 	}
     }
@@ -290,7 +305,7 @@ parse_line_table (struct dwarf1_debug* stash, struct dwarf1_unit* aUnit)
     }
 
   xptr = stash->line_section + aUnit->stmt_list_offset;
-  if (xptr < stash->line_section_end)
+  if (xptr + 8 <= stash->line_section_end)
     {
       unsigned long eachLine;
       bfd_byte *tblend;
@@ -318,6 +333,11 @@ parse_line_table (struct dwarf1_debug* stash, struct dwarf1_unit* aUnit)
 
       for (eachLine = 0; eachLine < aUnit->line_count; eachLine++)
 	{
+	  if (xptr + 10 > stash->line_section_end)
+	    {
+	      aUnit->line_count = eachLine;
+	      break;
+	    }
 	  /* A line number.  */
 	  aUnit->linenumber_table[eachLine].linenumber
 	    = bfd_get_32 (stash->abfd, (bfd_byte *) xptr);