[binutils-gdb] PR27845, readelf heap-buffer-overflow

[Alan Modra]

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

commit f2f9554bf0d15a5b93289ebfef7e04d0aeb51a60
Author: Alan Modra <amodra@gmail.com>
Date:   Tue May 11 17:57:59 2021 +0930

    PR27845, readelf heap-buffer-overflow
    
            PR 27845
            * dwarf.c (process_abbrev_set): Replace start and end parameters
            with section, abbrev_base, abbrev_size, abbrev_offset.  Update
            all callers.  Sanity check parameters correctly and emit warnings
            here rather than..
            (process_debug_info): ..here.

Diff:
---
 binutils/ChangeLog |  9 ++++++
 binutils/dwarf.c   | 80 ++++++++++++++++++++++++++++--------------------------
 2 files changed, 51 insertions(+), 38 deletions(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 38ae189daba..6703b16b2b0 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,12 @@
+2021-05-11  Alan Modra  <amodra@gmail.com>
+
+	PR 27845
+	* dwarf.c (process_abbrev_set): Replace start and end parameters
+	with section, abbrev_base, abbrev_size, abbrev_offset.  Update
+	all callers.  Sanity check parameters correctly and emit warnings
+	here rather than..
+	(process_debug_info): ..here.
+
 2021-05-10  Thomas Wolff  <towo@towo.net>
 
 	PR 4356
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index c584f5b2a24..aa48f69bbd4 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -1059,10 +1059,34 @@ add_abbrev_attr (unsigned long   attribute,
    an abbreviation set was found.  */
 
 static unsigned char *
-process_abbrev_set (unsigned char *        start,
-		    const unsigned char *  end,
-		    abbrev_list *          list)
+process_abbrev_set (struct dwarf_section *section,
+		    dwarf_vma abbrev_base,
+		    dwarf_vma abbrev_size,
+		    dwarf_vma abbrev_offset,
+		    abbrev_list *list)
 {
+  if (abbrev_base >= section->size
+      || abbrev_size > section->size - abbrev_base)
+    {
+      /* PR 17531: file:4bcd9ce9.  */
+      warn (_("Debug info is corrupted, abbrev size (%lx) is larger than "
+	      "abbrev section size (%lx)\n"),
+	      (unsigned long) abbrev_base + abbrev_size,
+	      (unsigned long) section->size);
+      return NULL;
+    }
+  if (abbrev_offset >= abbrev_size)
+    {
+      warn (_("Debug info is corrupted, abbrev offset (%lx) is larger than "
+	      "abbrev section size (%lx)\n"),
+	    (unsigned long) abbrev_offset,
+	    (unsigned long) abbrev_size);
+      return NULL;
+    }
+
+  unsigned char *start = section->start + abbrev_base;
+  unsigned char *end = start + abbrev_size;
+  start += abbrev_offset;
   while (start < end)
     {
       unsigned long entry;
@@ -3679,12 +3703,9 @@ process_debug_info (struct dwarf_section * section,
 
 	  list = new_abbrev_list (abbrev_base,
 				  compunit.cu_abbrev_offset);
-	  next = process_abbrev_set
-	    (((unsigned char *) debug_displays [abbrev_sec].section.start
-	      + abbrev_base + compunit.cu_abbrev_offset),
-	     ((unsigned char *) debug_displays [abbrev_sec].section.start
-	      + abbrev_base + abbrev_size),
-	     list);
+	  next = process_abbrev_set (&debug_displays[abbrev_sec].section,
+				     abbrev_base, abbrev_size,
+				     compunit.cu_abbrev_offset, list);
 	  list->start_of_next_abbrevs = next;
 	}
 
@@ -3905,34 +3926,18 @@ process_debug_info (struct dwarf_section * section,
 	}
 
       /* Process the abbrevs used by this compilation unit.  */
-      if (compunit.cu_abbrev_offset >= abbrev_size)
-	warn (_("Debug info is corrupted, abbrev offset (%lx) is larger than abbrev section size (%lx)\n"),
-	      (unsigned long) compunit.cu_abbrev_offset,
-	      (unsigned long) abbrev_size);
-      /* PR 17531: file:4bcd9ce9.  */
-      else if ((abbrev_base + abbrev_size)
-	       > debug_displays [abbrev_sec].section.size)
-	warn (_("Debug info is corrupted, abbrev size (%lx) is larger than abbrev section size (%lx)\n"),
-	      (unsigned long) abbrev_base + abbrev_size,
-	      (unsigned long) debug_displays [abbrev_sec].section.size);
-      else
+      list = find_abbrev_list_by_abbrev_offset (abbrev_base,
+						compunit.cu_abbrev_offset);
+      if (list == NULL)
 	{
-	  list = find_abbrev_list_by_abbrev_offset (abbrev_base,
-						    compunit.cu_abbrev_offset);
-	  if (list == NULL)
-	    {
-	      unsigned char * next;
-
-	      list = new_abbrev_list (abbrev_base,
-				      compunit.cu_abbrev_offset);
-	      next = process_abbrev_set
-		(((unsigned char *) debug_displays [abbrev_sec].section.start
-		  + abbrev_base + compunit.cu_abbrev_offset),
-		 ((unsigned char *) debug_displays [abbrev_sec].section.start
-		  + abbrev_base + abbrev_size),
-		 list);
-	      list->start_of_next_abbrevs = next;
-	    }
+	  unsigned char *next;
+
+	  list = new_abbrev_list (abbrev_base,
+				  compunit.cu_abbrev_offset);
+	  next = process_abbrev_set (&debug_displays[abbrev_sec].section,
+				     abbrev_base, abbrev_size,
+				     compunit.cu_abbrev_offset, list);
+	  list->start_of_next_abbrevs = next;
 	}
 
       level = 0;
@@ -6326,7 +6331,6 @@ display_debug_abbrev (struct dwarf_section *section,
 {
   abbrev_entry *entry;
   unsigned char *start = section->start;
-  const unsigned char *end = start + section->size;
 
   introduce (section, false);
 
@@ -6340,7 +6344,7 @@ display_debug_abbrev (struct dwarf_section *section,
       if (list == NULL)
 	{
 	  list = new_abbrev_list (0, offset);
-	  start = process_abbrev_set (start, end, list);
+	  start = process_abbrev_set (section, 0, section->size, offset, list);
 	  list->start_of_next_abbrevs = start;
 	}
       else