public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 00/19] Pointer UB in binutils/dwarf.c
@ 2021-05-15  8:09 Alan Modra
  2021-05-15  8:09 ` [PATCH 01/19] _mul_overflow and get_encoded_value Alan Modra
                   ` (18 more replies)
  0 siblings, 19 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

This series of patches is aimed at removing pointer undefined behaviour
as per ISO C 1989/1999 sections 6.5.6 and 6.5.8, in dwarf.c.  Most of
these are probably benign with current compilers, but I do worry about
some future crazy compiler optimisation making use of the UB to omit
some of the pointer bound checking.

Alan Modra (19):
  _mul_overflow and get_encoded_value
  SAFE_BYTE_GET_INTERNAL
  process_debug_info
  read_debug_line_header
  display_debug_lines_decoded
  display_debug_pubnames_worker
  display_debug_macinfo
  get_line_filename_and_dirname
  display_debug_macro
  display_loc_list
  display_debug_aranges
  display_debug_str_offsets
  display_debug_rnglists_list
  display_debug_ranges
  read_cie
  display_debug_frames
  display_debug_names
  display_gdb_index
  process_cu_tu_index

 binutils/ChangeLog | 102 +++++++
 binutils/bucomm.h  |   8 +
 binutils/dwarf.c   | 688 ++++++++++++++++++++++-----------------------
 3 files changed, 447 insertions(+), 351 deletions(-)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/19] _mul_overflow and get_encoded_value
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 02/19] SAFE_BYTE_GET_INTERNAL Alan Modra
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

A sufficiently mad compiler optimiser can take undefined behaviour
according to the C standard as an opportunity to remove code.  Since
"data + size" might be seen to be past the end of an array,
calculating such an expression is UB.

_mul_overflow is infrastructure for later patches.

	* bucomm.h (_mul_overflow): Define.
	* dwarf.c (get_encoded_value): Avoid pointer UB.

diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index 78f61762cac..2769c278671 100644
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -80,4 +80,12 @@ void *xmalloc (size_t);
 
 void *xrealloc (void *, size_t);
 
+#if __GNUC__ >= 7
+#define _mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
+#else
+/* Assumes unsigned values.  Careful!  Args evaluated multiple times.  */
+#define _mul_overflow(a, b, res) \
+  ((*res) = (a), (*res) *= (b), (b) != 0 && (*res) / (b) != (a))
+#endif
+
 #endif /* _BUCOMM_H */
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 2794a15a1d3..020b7e071ec 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -178,7 +178,7 @@ get_encoded_value (unsigned char **pdata,
   unsigned int size = size_of_encoded_value (encoding);
   dwarf_vma val;
 
-  if (data + size >= end)
+  if (data >= end || size > (size_t) (end - data))
     {
       warn (_("Encoded value extends past end of section\n"));
       * pdata = end;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 02/19] SAFE_BYTE_GET_INTERNAL
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
  2021-05-15  8:09 ` [PATCH 01/19] _mul_overflow and get_encoded_value Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 03/19] process_debug_info Alan Modra
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

We won't want this assert triggering in the next release.

	* dwarf.c (SAFE_BYTE_GET_INTERNAL): Assert only when ENABLE_CHECKING.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 020b7e071ec..3a1c18e082f 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -390,8 +390,11 @@ read_leb128 (unsigned char *data,
 		 (int) amount, (int) sizeof (VAL));			\
 	  amount = sizeof (VAL);					\
 	}								\
-      assert ((PTR) <= (END));						\
+      if (ENABLE_CHECKING)						\
+	assert ((PTR) <= (END));					\
       size_t avail = (END) - (PTR);					\
+      if ((PTR) > (END))						\
+	avail = 0;							\
       if (amount > avail)						\
 	amount = avail;							\
       if (amount == 0)							\

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 03/19] process_debug_info
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
  2021-05-15  8:09 ` [PATCH 01/19] _mul_overflow and get_encoded_value Alan Modra
  2021-05-15  8:09 ` [PATCH 02/19] SAFE_BYTE_GET_INTERNAL Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 04/19] read_debug_line_header Alan Modra
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

This patch constrains process_debug_info to stay within the data
specified by the CU length rather than allowing access up to the end
of the section.

	* dwarf.c (process_debug_info): Always do the first CU length
	scan for sanity checks.  Remove initial_length_size var and
	instead calculate end_cu.  Use end_cu to limit data reads.
	Delete now dead code checking length.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 3a1c18e082f..b7061a9b99c 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -3429,47 +3429,49 @@ process_debug_info (struct dwarf_section * section,
   unsigned int unit;
   unsigned int num_units = 0;
 
-  if ((do_loc || do_debug_loc || do_debug_ranges)
-      && num_debug_info_entries == 0
-      && ! do_types)
+  /* First scan the section to get the number of comp units.
+     Length sanity checks are done here.  */
+  for (section_begin = start, num_units = 0; section_begin < end;
+       num_units ++)
     {
       dwarf_vma length;
 
-      /* First scan the section to get the number of comp units.  */
-      for (section_begin = start, num_units = 0; section_begin < end;
-	   num_units ++)
-	{
-	  /* Read the first 4 bytes.  For a 32-bit DWARF section, this
-	     will be the length.  For a 64-bit DWARF section, it'll be
-	     the escape code 0xffffffff followed by an 8 byte length.  */
-	  SAFE_BYTE_GET_AND_INC (length, section_begin, 4, end);
+      /* Read the first 4 bytes.  For a 32-bit DWARF section, this
+	 will be the length.  For a 64-bit DWARF section, it'll be
+	 the escape code 0xffffffff followed by an 8 byte length.  */
+      SAFE_BYTE_GET_AND_INC (length, section_begin, 4, end);
 
-	  if (length == 0xffffffff)
-	    SAFE_BYTE_GET_AND_INC (length, section_begin, 8, end);
-	  else if (length >= 0xfffffff0 && length < 0xffffffff)
-	    {
-	      warn (_("Reserved length value (0x%s) found in section %s\n"),
-		    dwarf_vmatoa ("x", length), section->name);
-	      return false;
-	    }
-
-	  /* Negative values are illegal, they may even cause infinite
-	     looping.  This can happen if we can't accurately apply
-	     relocations to an object file, or if the file is corrupt.  */
-	  if (length > (size_t) (end - section_begin))
-	    {
-	      warn (_("Corrupt unit length (0x%s) found in section %s\n"),
-		    dwarf_vmatoa ("x", length), section->name);
-	      return false;
-	    }
-	  section_begin += length;
+      if (length == 0xffffffff)
+	SAFE_BYTE_GET_AND_INC (length, section_begin, 8, end);
+      else if (length >= 0xfffffff0 && length < 0xffffffff)
+	{
+	  warn (_("Reserved length value (0x%s) found in section %s\n"),
+		dwarf_vmatoa ("x", length), section->name);
+	  return false;
 	}
 
-      if (num_units == 0)
+      /* Negative values are illegal, they may even cause infinite
+	 looping.  This can happen if we can't accurately apply
+	 relocations to an object file, or if the file is corrupt.  */
+      if (length > (size_t) (end - section_begin))
 	{
-	  error (_("No comp units in %s section ?\n"), section->name);
+	  warn (_("Corrupt unit length (0x%s) found in section %s\n"),
+		dwarf_vmatoa ("x", length), section->name);
 	  return false;
 	}
+      section_begin += length;
+    }
+
+  if (num_units == 0)
+    {
+      error (_("No comp units in %s section ?\n"), section->name);
+      return false;
+    }
+
+  if ((do_loc || do_debug_loc || do_debug_ranges)
+      && num_debug_info_entries == 0
+      && ! do_types)
+    {
 
       /* Then allocate an array to hold the information.  */
       debug_information = (debug_info *) cmalloc (num_units,
@@ -3530,11 +3532,12 @@ process_debug_info (struct dwarf_section * section,
       size_t                    abbrev_size;
       dwarf_vma                 cu_offset;
       unsigned int              offset_size;
-      unsigned int              initial_length_size;
       struct cu_tu_set *        this_set;
       abbrev_list *             list;
+      unsigned char *end_cu;
 
       hdrptr = start;
+      cu_offset = start - section_begin;
 
       SAFE_BYTE_GET_AND_INC (compunit.cu_length, hdrptr, 4, end);
 
@@ -3542,17 +3545,12 @@ process_debug_info (struct dwarf_section * section,
 	{
 	  SAFE_BYTE_GET_AND_INC (compunit.cu_length, hdrptr, 8, end);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
+	offset_size = 4;
+      end_cu = hdrptr + compunit.cu_length;
 
-      SAFE_BYTE_GET_AND_INC (compunit.cu_version, hdrptr, 2, end);
-
-      cu_offset = start - section_begin;
+      SAFE_BYTE_GET_AND_INC (compunit.cu_version, hdrptr, 2, end_cu);
 
       this_set = find_cu_tu_set_v2 (cu_offset, do_types);
 
@@ -3564,19 +3562,20 @@ process_debug_info (struct dwarf_section * section,
 	}
       else
 	{
-	  SAFE_BYTE_GET_AND_INC (compunit.cu_unit_type, hdrptr, 1, end);
+	  SAFE_BYTE_GET_AND_INC (compunit.cu_unit_type, hdrptr, 1, end_cu);
 	  do_types = (compunit.cu_unit_type == DW_UT_type);
 
-	  SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end);
+	  SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end_cu);
 	}
 
-      SAFE_BYTE_GET_AND_INC (compunit.cu_abbrev_offset, hdrptr, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (compunit.cu_abbrev_offset, hdrptr, offset_size,
+			     end_cu);
 
       if (compunit.cu_unit_type == DW_UT_split_compile
 	  || compunit.cu_unit_type == DW_UT_skeleton)
 	{
 	  uint64_t dwo_id;
-	  SAFE_BYTE_GET_AND_INC (dwo_id, hdrptr, 8, end);
+	  SAFE_BYTE_GET_AND_INC (dwo_id, hdrptr, 8, end_cu);
 	}
 
       if (this_set == NULL)
@@ -3604,8 +3603,7 @@ process_debug_info (struct dwarf_section * section,
 	  list->start_of_next_abbrevs = next;
 	}
 
-      start = section_begin + cu_offset + compunit.cu_length
-	+ initial_length_size;
+      start = end_cu;
       record_abbrev_list_for_cu (cu_offset, start - section_begin, list);
     }
 
@@ -3616,17 +3614,17 @@ process_debug_info (struct dwarf_section * section,
       unsigned char *tags;
       int level, last_level, saved_level;
       dwarf_vma cu_offset;
-      unsigned long sec_off;
       unsigned int offset_size;
-      unsigned int initial_length_size;
       dwarf_vma signature = 0;
       dwarf_vma type_offset = 0;
       struct cu_tu_set *this_set;
       dwarf_vma abbrev_base;
       size_t abbrev_size;
       abbrev_list * list = NULL;
+      unsigned char *end_cu;
 
       hdrptr = start;
+      cu_offset = start - section_begin;
 
       SAFE_BYTE_GET_AND_INC (compunit.cu_length, hdrptr, 4, end);
 
@@ -3634,17 +3632,12 @@ process_debug_info (struct dwarf_section * section,
 	{
 	  SAFE_BYTE_GET_AND_INC (compunit.cu_length, hdrptr, 8, end);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
-
-      SAFE_BYTE_GET_AND_INC (compunit.cu_version, hdrptr, 2, end);
+	offset_size = 4;
+      end_cu = hdrptr + compunit.cu_length;
 
-      cu_offset = start - section_begin;
+      SAFE_BYTE_GET_AND_INC (compunit.cu_version, hdrptr, 2, end_cu);
 
       this_set = find_cu_tu_set_v2 (cu_offset, do_types);
 
@@ -3656,13 +3649,13 @@ process_debug_info (struct dwarf_section * section,
 	}
       else
 	{
-	  SAFE_BYTE_GET_AND_INC (compunit.cu_unit_type, hdrptr, 1, end);
+	  SAFE_BYTE_GET_AND_INC (compunit.cu_unit_type, hdrptr, 1, end_cu);
 	  do_types = (compunit.cu_unit_type == DW_UT_type);
 
-	  SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end);
+	  SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end_cu);
 	}
 
-      SAFE_BYTE_GET_AND_INC (compunit.cu_abbrev_offset, hdrptr, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (compunit.cu_abbrev_offset, hdrptr, offset_size, end_cu);
 
       if (this_set == NULL)
 	{
@@ -3676,14 +3669,14 @@ process_debug_info (struct dwarf_section * section,
 	}
 
       if (compunit.cu_version < 5)
-	SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end);
+	SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end_cu);
 
       bool do_dwo_id = false;
       uint64_t dwo_id = 0;
       if (compunit.cu_unit_type == DW_UT_split_compile
 	  || compunit.cu_unit_type == DW_UT_skeleton)
 	{
-	  SAFE_BYTE_GET_AND_INC (dwo_id, hdrptr, 8, end);
+	  SAFE_BYTE_GET_AND_INC (dwo_id, hdrptr, 8, end_cu);
 	  do_dwo_id = true;
 	}
 
@@ -3697,15 +3690,13 @@ process_debug_info (struct dwarf_section * section,
 
       if (do_types)
 	{
-	  SAFE_BYTE_GET_AND_INC (signature, hdrptr, 8, end);
-	  SAFE_BYTE_GET_AND_INC (type_offset, hdrptr, offset_size, end);
+	  SAFE_BYTE_GET_AND_INC (signature, hdrptr, 8, end_cu);
+	  SAFE_BYTE_GET_AND_INC (type_offset, hdrptr, offset_size, end_cu);
 	}
 
-      if (dwarf_start_die > (cu_offset + compunit.cu_length
-			     + initial_length_size))
+      if (dwarf_start_die >= (size_t) (end_cu - section_begin))
 	{
-	  start = section_begin + cu_offset + compunit.cu_length
-	    + initial_length_size;
+	  start = end_cu;
 	  continue;
 	}
 
@@ -3780,20 +3771,8 @@ process_debug_info (struct dwarf_section * section,
 	    }
 	}
 
-      sec_off = cu_offset + initial_length_size;
-      if (sec_off + compunit.cu_length < sec_off
-	  || sec_off + compunit.cu_length > section->size)
-	{
-	  warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
-		section->name,
-		(unsigned long) cu_offset,
-		dwarf_vmatoa ("x", compunit.cu_length));
-	  num_units = unit;
-	  break;
-	}
-
       tags = hdrptr;
-      start += compunit.cu_length + initial_length_size;
+      start = end_cu;
 
       if (compunit.cu_version < 2 || compunit.cu_version > 5)
 	{
@@ -3967,7 +3946,7 @@ process_debug_info (struct dwarf_section * section,
 					    attr->implicit_const,
 					    section_begin,
 					    tags,
-					    end,
+					    start,
 					    cu_offset,
 					    compunit.cu_pointer_size,
 					    offset_size,

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 04/19] read_debug_line_header
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (2 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 03/19] process_debug_info Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 05/19] display_debug_lines_decoded Alan Modra
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

This patch also better constrains the data read, and removes pointer UB.

	* dwarf.c (read_debug_line_header): Delete initial_length_size.
	Avoid pointer UB.  Keep within length specified by header.
	Delete dead code.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index b7061a9b99c..4f69dbb8f85 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -4053,7 +4053,6 @@ read_debug_line_header (struct dwarf_section * section,
 			unsigned char ** end_of_sequence)
 {
   unsigned char *hdrptr;
-  unsigned int initial_length_size;
 
   /* Extract information from the Line Number Program Header.
      (section 6.2.4 in the Dwarf3 doc).  */
@@ -4067,15 +4066,11 @@ read_debug_line_header (struct dwarf_section * section,
       /* This section is 64-bit DWARF 3.  */
       SAFE_BYTE_GET_AND_INC (linfo->li_length, hdrptr, 8, end);
       linfo->li_offset_size = 8;
-      initial_length_size = 12;
     }
   else
-    {
-      linfo->li_offset_size = 4;
-      initial_length_size = 4;
-    }
+    linfo->li_offset_size = 4;
 
-  if (linfo->li_length + initial_length_size > section->size)
+  if (linfo->li_length > (size_t) (end - hdrptr))
     {
       /* If the length field has a relocation against it, then we should
 	 not complain if it is inaccurate (and probably negative).  This
@@ -4085,7 +4080,7 @@ read_debug_line_header (struct dwarf_section * section,
 	 is used to compute the correct length once that is done.  */
       if (reloc_at (section, (hdrptr - section->start) - linfo->li_offset_size))
 	{
-	  linfo->li_length = (end - data) - initial_length_size;
+	  linfo->li_length = end - hdrptr;
 	}
       else
 	{
@@ -4094,6 +4089,7 @@ read_debug_line_header (struct dwarf_section * section,
 	  return NULL;
 	}
     }
+  end = hdrptr + linfo->li_length;
 
   /* Get and check the version number.  */
   SAFE_BYTE_GET_AND_INC (linfo->li_version, hdrptr, 2, end);
@@ -4144,16 +4140,7 @@ read_debug_line_header (struct dwarf_section * section,
   SAFE_BYTE_GET_AND_INC (linfo->li_line_range, hdrptr, 1, end);
   SAFE_BYTE_GET_AND_INC (linfo->li_opcode_base, hdrptr, 1, end);
 
-  * end_of_sequence = data + linfo->li_length + initial_length_size;
-  /* PR 17512: file:002-117414-0.004.  */
-  if (* end_of_sequence > end)
-    {
-      warn (_("Line length %s extends beyond end of section\n"),
-	    dwarf_vmatoa ("u", linfo->li_length));
-      * end_of_sequence = end;
-      return NULL;
-    }
-
+  *end_of_sequence = end;
   return hdrptr;
 }
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 05/19] display_debug_lines_decoded
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (3 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 04/19] read_debug_line_header Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 06/19] display_debug_pubnames_worker Alan Modra
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

The directory_table strnlen used the negative of the proper size.  After
fixing that I realised we don't need strnlen here.

	* dwarf.c (display_debug_lines_decoded): Don't use strnlen when
	we have already checked for NUL termination.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 4f69dbb8f85..e881ceecb79 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -5042,8 +5042,8 @@ display_debug_lines_decoded (struct dwarf_section *  section,
 		  while (*ptr_directory_table != 0)
 		    {
 		      directory_table[i] = ptr_directory_table;
-		      ptr_directory_table += strnlen ((char *) ptr_directory_table,
-						      ptr_directory_table - end) + 1;
+		      ptr_directory_table
+			+= strlen ((char *) ptr_directory_table) + 1;
 		      i++;
 		    }
 		}
@@ -5082,8 +5082,8 @@ display_debug_lines_decoded (struct dwarf_section *  section,
 		  while (*ptr_file_name_table != 0)
 		    {
 		      file_table[i].name = ptr_file_name_table;
-		      ptr_file_name_table += strnlen ((char *) ptr_file_name_table,
-						      end - ptr_file_name_table) + 1;
+		      ptr_file_name_table
+			+= strlen ((char *) ptr_file_name_table) + 1;
 
 		      /* We are not interested in directory, time or size.  */
 		      READ_ULEB (file_table[i].directory_index,

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 06/19] display_debug_pubnames_worker
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (4 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 05/19] display_debug_lines_decoded Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 07/19] display_debug_macinfo Alan Modra
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_pubnames_worker): Delete initial_length_size.
	Simplify length check.  Constrain reads to length given by header.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index e881ceecb79..878f4f766db 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -5616,29 +5616,23 @@ display_debug_pubnames_worker (struct dwarf_section *section,
   while (start < end)
     {
       unsigned char *data;
-      unsigned long sec_off;
-      unsigned int offset_size, initial_length_size;
+      unsigned long sec_off = start - section->start;
+      unsigned int offset_size;
 
       SAFE_BYTE_GET_AND_INC (names.pn_length, start, 4, end);
       if (names.pn_length == 0xffffffff)
 	{
 	  SAFE_BYTE_GET_AND_INC (names.pn_length, start, 8, end);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
+	offset_size = 4;
 
-      sec_off = start - section->start;
-      if (sec_off + names.pn_length < sec_off
-	  || sec_off + names.pn_length > section->size)
+      if (names.pn_length > (size_t) (end - start))
 	{
 	  warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
 		section->name,
-		sec_off - initial_length_size,
+		sec_off,
 		dwarf_vmatoa ("x", names.pn_length));
 	  break;
 	}
@@ -5646,8 +5640,8 @@ display_debug_pubnames_worker (struct dwarf_section *section,
       data = start;
       start += names.pn_length;
 
-      SAFE_BYTE_GET_AND_INC (names.pn_version, data, 2, end);
-      SAFE_BYTE_GET_AND_INC (names.pn_offset, data, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (names.pn_version, data, 2, start);
+      SAFE_BYTE_GET_AND_INC (names.pn_offset, data, offset_size, start);
 
       if (num_debug_info_entries != DEBUG_INFO_UNAVAILABLE
 	  && num_debug_info_entries > 0
@@ -5655,7 +5649,7 @@ display_debug_pubnames_worker (struct dwarf_section *section,
 	warn (_(".debug_info offset of 0x%lx in %s section does not point to a CU header.\n"),
 	      (unsigned long) names.pn_offset, section->name);
 
-      SAFE_BYTE_GET_AND_INC (names.pn_size, data, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (names.pn_size, data, offset_size, start);
 
       printf (_("  Length:                              %ld\n"),
 	      (long) names.pn_length);
@@ -5689,14 +5683,14 @@ display_debug_pubnames_worker (struct dwarf_section *section,
 	  bfd_size_type maxprint;
 	  dwarf_vma offset;
 
-	  SAFE_BYTE_GET_AND_INC (offset, data, offset_size, end);
+	  SAFE_BYTE_GET_AND_INC (offset, data, offset_size, start);
 
 	  if (offset == 0)
 	    break;
 
-	  if (data >= end)
+	  if (data >= start)
 	    break;
-	  maxprint = (end - data) - 1;
+	  maxprint = (start - data) - 1;
 
 	  if (is_gnu)
 	    {
@@ -5705,7 +5699,7 @@ display_debug_pubnames_worker (struct dwarf_section *section,
 	      const char *kind_name;
 	      int is_static;
 
-	      SAFE_BYTE_GET_AND_INC (kind_data, data, 1, end);
+	      SAFE_BYTE_GET_AND_INC (kind_data, data, 1, start);
 	      maxprint --;
 	      /* GCC computes the kind as the upper byte in the CU index
 		 word, and then right shifts it by the CU index size.
@@ -5724,9 +5718,9 @@ display_debug_pubnames_worker (struct dwarf_section *section,
 		    (unsigned long) offset, (int) maxprint, data);
 
 	  data += strnlen ((char *) data, maxprint);
-	  if (data < end)
+	  if (data < start)
 	    data++;
-	  if (data >= end)
+	  if (data >= start)
 	    break;
 	}
     }

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 07/19] display_debug_macinfo
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (5 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 06/19] display_debug_pubnames_worker Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 08/19] get_line_filename_and_dirname Alan Modra
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

The existing code went to the bother of using strnlen for scanning but
went wild when printing, and possibly incremented curr past end.

	* dwarf.c (display_debug_macinfo): Print strings that might not
	be zero terminated with %*s.  Don't bump curr if unterminated.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 878f4f766db..d184e5289ed 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -5780,17 +5780,21 @@ display_debug_macinfo (struct dwarf_section *section,
 	case DW_MACINFO_define:
 	  READ_ULEB (lineno, curr, end);
 	  string = curr;
-	  curr += strnlen ((char *) string, end - string) + 1;
-	  printf (_(" DW_MACINFO_define - lineno : %d macro : %s\n"),
-		  lineno, string);
+	  curr += strnlen ((char *) string, end - string);
+	  printf (_(" DW_MACINFO_define - lineno : %d macro : %*s\n"),
+		  lineno, (int) (curr - string), string);
+	  if (curr < end)
+	    curr++;
 	  break;
 
 	case DW_MACINFO_undef:
 	  READ_ULEB (lineno, curr, end);
 	  string = curr;
-	  curr += strnlen ((char *) string, end - string) + 1;
-	  printf (_(" DW_MACINFO_undef - lineno : %d macro : %s\n"),
-		  lineno, string);
+	  curr += strnlen ((char *) string, end - string);
+	  printf (_(" DW_MACINFO_undef - lineno : %d macro : %*s\n"),
+		  lineno, (int) (curr - string), string);
+	  if (curr < end)
+	    curr++;
 	  break;
 
 	case DW_MACINFO_vendor_ext:
@@ -5799,9 +5803,11 @@ display_debug_macinfo (struct dwarf_section *section,
 
 	    READ_ULEB (constant, curr, end);
 	    string = curr;
-	    curr += strnlen ((char *) string, end - string) + 1;
-	    printf (_(" DW_MACINFO_vendor_ext - constant : %d string : %s\n"),
-		    constant, string);
+	    curr += strnlen ((char *) string, end - string);
+	    printf (_(" DW_MACINFO_vendor_ext - constant : %d string : %*s\n"),
+		    constant, (int) (curr - string), string);
+	    if (curr < end)
+	      curr++;
 	  }
 	  break;
 	}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 08/19] get_line_filename_and_dirname
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (6 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 07/19] display_debug_macinfo Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 09/19] display_debug_macro Alan Modra
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (get_line_filename_and_dirname): Delete initial_length_size.
	Simplify length sanity check, and check for too small lengths.
	Constrain data reads to header length.  Avoid pointer UB.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index d184e5289ed..9d782912208 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -5827,7 +5827,7 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
 {
   struct dwarf_section *section = &debug_displays [line].section;
   unsigned char *hdrptr, *dirtable, *file_name;
-  unsigned int offset_size, initial_length_size;
+  unsigned int offset_size;
   unsigned int version, opcode_base;
   dwarf_vma length, diridx;
   const unsigned char * end;
@@ -5847,16 +5847,14 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
       /* This section is 64-bit DWARF 3.  */
       SAFE_BYTE_GET_AND_INC (length, hdrptr, 8, end);
       offset_size = 8;
-      initial_length_size = 12;
     }
   else
-    {
-      offset_size = 4;
-      initial_length_size = 4;
-    }
-  if (length + initial_length_size < length
-      || length + initial_length_size > section->size)
+    offset_size = 4;
+
+  if (length > (size_t) (end - hdrptr)
+      || length < 2 + offset_size + 1 + 3 + 1)
     return NULL;
+  end = hdrptr + length;
 
   SAFE_BYTE_GET_AND_INC (version, hdrptr, 2, end);
   if (version != 2 && version != 3 && version != 4)
@@ -5867,18 +5865,19 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
   hdrptr += 3;		    /* Skip default_is_stmt, line_base, line_range.  */
 
   SAFE_BYTE_GET_AND_INC (opcode_base, hdrptr, 1, end);
-  if (opcode_base == 0)
+  if (opcode_base == 0
+      || opcode_base - 1 >= (size_t) (end - hdrptr))
     return NULL;
 
   hdrptr += opcode_base - 1;
-  if (hdrptr >= end)
-    return NULL;
 
   dirtable = hdrptr;
   /* Skip over dirname table.  */
   while (*hdrptr != '\0')
     {
-      hdrptr += strnlen ((char *) hdrptr, end - hdrptr) + 1;
+      hdrptr += strnlen ((char *) hdrptr, end - hdrptr);
+      if (hdrptr < end)
+	hdrptr++;
       if (hdrptr >= end)
 	return NULL;
     }
@@ -5887,7 +5886,9 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
   /* Now skip over preceding filename table entries.  */
   for (; hdrptr < end && *hdrptr != '\0' && fileidx > 1; fileidx--)
     {
-      hdrptr += strnlen ((char *) hdrptr, end - hdrptr) + 1;
+      hdrptr += strnlen ((char *) hdrptr, end - hdrptr);
+      if (hdrptr < end)
+	hdrptr++;
       SKIP_ULEB (hdrptr, end);
       SKIP_ULEB (hdrptr, end);
       SKIP_ULEB (hdrptr, end);
@@ -5896,14 +5897,20 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
     return NULL;
 
   file_name = hdrptr;
-  hdrptr += strnlen ((char *) hdrptr, end - hdrptr) + 1;
+  hdrptr += strnlen ((char *) hdrptr, end - hdrptr);
+  if (hdrptr < end)
+    hdrptr++;
   if (hdrptr >= end)
     return NULL;
   READ_ULEB (diridx, hdrptr, end);
   if (diridx == 0)
     return file_name;
   for (; dirtable < end && *dirtable != '\0' && diridx > 1; diridx--)
-    dirtable += strnlen ((char *) dirtable, end - dirtable) + 1;
+    {
+      dirtable += strnlen ((char *) dirtable, end - dirtable);
+      if (dirtable < end)
+	dirtable++;
+    }
   if (dirtable >= end || *dirtable == '\0')
     return NULL;
   *dir_name = dirtable;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 09/19] display_debug_macro
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (7 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 08/19] get_line_filename_and_dirname Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 10/19] display_loc_list Alan Modra
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_macro): Print strings that might not
	be zero terminated with %*s.  Don't bump curr if unterminated.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 9d782912208..68732cf491b 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -6042,17 +6042,21 @@ display_debug_macro (struct dwarf_section *section,
 	    case DW_MACRO_define:
 	      READ_ULEB (lineno, curr, end);
 	      string = curr;
-	      curr += strnlen ((char *) string, end - string) + 1;
-	      printf (_(" DW_MACRO_define - lineno : %d macro : %s\n"),
-		      lineno, string);
+	      curr += strnlen ((char *) string, end - string);
+	      printf (_(" DW_MACRO_define - lineno : %d macro : %*s\n"),
+		      lineno, (int) (curr - string), string);
+	      if (curr < end)
+		curr++;
 	      break;
 
 	    case DW_MACRO_undef:
 	      READ_ULEB (lineno, curr, end);
 	      string = curr;
-	      curr += strnlen ((char *) string, end - string) + 1;
-	      printf (_(" DW_MACRO_undef - lineno : %d macro : %s\n"),
-		      lineno, string);
+	      curr += strnlen ((char *) string, end - string);
+	      printf (_(" DW_MACRO_undef - lineno : %d macro : %*s\n"),
+		      lineno, (int) (curr - string), string);
+	      if (curr < end)
+		curr++;
 	      break;
 
 	    case DW_MACRO_start_file:

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 10/19] display_loc_list
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (8 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 09/19] display_debug_macro Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 11/19] display_debug_aranges Alan Modra
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_loc_list): Avoid pointer UB.  Correct check
	before reading uleb length.  Warn on excess length.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 68732cf491b..4d29591faa6 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -6355,7 +6355,7 @@ display_loc_list (struct dwarf_section *section,
       dwarf_vma off = offset + (start - *start_ptr);
       dwarf_vma vbegin = vm1, vend = vm1;
 
-      if (start + 2 * pointer_size > section_end)
+      if (2 * pointer_size > (size_t) (section_end - start))
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);
@@ -6408,7 +6408,7 @@ display_loc_list (struct dwarf_section *section,
 		  (unsigned long) off, 8, "");
 	}
 
-      if (start + 2 > section_end)
+      if (2 > (size_t) (section_end - start))
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);
@@ -6417,7 +6417,7 @@ display_loc_list (struct dwarf_section *section,
 
       SAFE_BYTE_GET_AND_INC (length, start, 2, section_end);
 
-      if (start + length > section_end)
+      if (length > (size_t) (section_end - start))
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);
@@ -6579,15 +6579,21 @@ display_loclists_list (struct dwarf_section *section,
 	  && llet != DW_LLE_start_length)
 	continue;
 
-      if (start + 2 > section_end)
+      if (start == section_end)
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);
 	  break;
 	}
-
       READ_ULEB (length, start, section_end);
 
+      if (length > (size_t) (section_end - start))
+	{
+	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
+		(unsigned long) offset);
+	  break;
+	}
+
       print_dwarf_vma (begin, pointer_size);
       print_dwarf_vma (end, pointer_size);
 
@@ -6751,7 +6757,7 @@ display_loc_list_dwo (struct dwarf_section *section,
 	  return;
 	}
 
-      if (start + 2 > section_end)
+      if (2 > (size_t) (section_end - start))
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);
@@ -6759,7 +6765,7 @@ display_loc_list_dwo (struct dwarf_section *section,
 	}
 
       SAFE_BYTE_GET_AND_INC (length, start, 2, section_end);
-      if (start + length > section_end)
+      if (length > (size_t) (section_end - start))
 	{
 	  warn (_("Location list starting at offset 0x%lx is not terminated.\n"),
 		(unsigned long) offset);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 11/19] display_debug_aranges
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (9 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 10/19] display_loc_list Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 12/19] display_debug_str_offsets Alan Modra
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_aranges): Delete initial_length_size.
	Use end_ranges to constrain data reads to header length.  Avoid
	pointer UB.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 4d29591faa6..cd76f3f5e83 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -7187,36 +7187,33 @@ display_debug_aranges (struct dwarf_section *section,
       unsigned char address_size;
       int excess;
       unsigned int offset_size;
-      unsigned int initial_length_size;
+      unsigned char *end_ranges;
 
       hdrptr = start;
+      sec_off = hdrptr - section->start;
 
       SAFE_BYTE_GET_AND_INC (arange.ar_length, hdrptr, 4, end);
       if (arange.ar_length == 0xffffffff)
 	{
 	  SAFE_BYTE_GET_AND_INC (arange.ar_length, hdrptr, 8, end);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
+	offset_size = 4;
 
-      sec_off = hdrptr - section->start;
-      if (sec_off + arange.ar_length < sec_off
-	  || sec_off + arange.ar_length > section->size)
+      if (arange.ar_length > (size_t) (end - hdrptr))
 	{
 	  warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
 		section->name,
-		sec_off - initial_length_size,
+		sec_off,
 		dwarf_vmatoa ("x", arange.ar_length));
 	  break;
 	}
+      end_ranges = hdrptr + arange.ar_length;
 
-      SAFE_BYTE_GET_AND_INC (arange.ar_version, hdrptr, 2, end);
-      SAFE_BYTE_GET_AND_INC (arange.ar_info_offset, hdrptr, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (arange.ar_version, hdrptr, 2, end_ranges);
+      SAFE_BYTE_GET_AND_INC (arange.ar_info_offset, hdrptr, offset_size,
+			     end_ranges);
 
       if (num_debug_info_entries != DEBUG_INFO_UNAVAILABLE
 	  && num_debug_info_entries > 0
@@ -7224,8 +7221,8 @@ display_debug_aranges (struct dwarf_section *section,
 	warn (_(".debug_info offset of 0x%lx in %s section does not point to a CU header.\n"),
 	      (unsigned long) arange.ar_info_offset, section->name);
 
-      SAFE_BYTE_GET_AND_INC (arange.ar_pointer_size, hdrptr, 1, end);
-      SAFE_BYTE_GET_AND_INC (arange.ar_segment_size, hdrptr, 1, end);
+      SAFE_BYTE_GET_AND_INC (arange.ar_pointer_size, hdrptr, 1, end_ranges);
+      SAFE_BYTE_GET_AND_INC (arange.ar_segment_size, hdrptr, 1, end_ranges);
 
       if (arange.ar_version != 2 && arange.ar_version != 3)
 	{
@@ -7277,12 +7274,12 @@ display_debug_aranges (struct dwarf_section *section,
       if (excess)
 	addr_ranges += (2 * address_size) - excess;
 
-      start += arange.ar_length + initial_length_size;
+      start = end_ranges;
 
-      while (addr_ranges + 2 * address_size <= start)
+      while (2 * address_size <= (size_t) (start - addr_ranges))
 	{
-	  SAFE_BYTE_GET_AND_INC (address, addr_ranges, address_size, end);
-	  SAFE_BYTE_GET_AND_INC (length, addr_ranges, address_size, end);
+	  SAFE_BYTE_GET_AND_INC (address, addr_ranges, address_size, start);
+	  SAFE_BYTE_GET_AND_INC (length, addr_ranges, address_size, start);
 
 	  printf ("    ");
 	  print_dwarf_vma (address, address_size);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 12/19] display_debug_str_offsets
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (10 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 11/19] display_debug_aranges Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 13/19] display_debug_rnglists_list Alan Modra
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_str_offsets): Constrain reads to length
	given in header.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index cd76f3f5e83..a0b84fc8d85 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -7480,12 +7480,12 @@ display_debug_str_offsets (struct dwarf_section *section,
 	    }
 
 	  int version;
-	  SAFE_BYTE_GET_AND_INC (version, curr, 2, end);
+	  SAFE_BYTE_GET_AND_INC (version, curr, 2, entries_end);
 	  if (version != 5)
 	    warn (_("Unexpected version number in str_offset header: %#x\n"), version);
 
 	  int padding;
-	  SAFE_BYTE_GET_AND_INC (padding, curr, 2, end);
+	  SAFE_BYTE_GET_AND_INC (padding, curr, 2, entries_end);
 	  if (padding != 0)
 	    warn (_("Unexpected value in str_offset header's padding field: %#x\n"), padding);
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 13/19] display_debug_rnglists_list
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (11 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 12/19] display_debug_str_offsets Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 14/19] display_debug_ranges Alan Modra
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_rnglists_list): Avoid pointer UB.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index a0b84fc8d85..c4b6edf8721 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -7603,7 +7603,7 @@ display_debug_rnglists_list (unsigned char *start, unsigned char *finish,
       /* Initialize it due to a false compiler warning.  */
       dwarf_vma begin = -1, length, end = -1;
 
-      if (start + 1 > finish)
+      if (start >= finish)
 	{
 	  warn (_("Range list starting at offset 0x%lx is not terminated.\n"),
 		offset);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 14/19] display_debug_ranges
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (12 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 13/19] display_debug_rnglists_list Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 15/19] read_cie Alan Modra
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_ranges): Delete initial_length_size.
	Correct fallback size calculated on finding a reloc.  Constrain
	data reads to length given in header.  Avoid pointer UB.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index c4b6edf8721..9243c853020 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -7691,7 +7691,6 @@ display_debug_ranges (struct dwarf_section *section,
   if (is_rnglists)
     {
       dwarf_vma initial_length;
-      unsigned int initial_length_size;
       unsigned char segment_selector_size;
       unsigned int offset_size, offset_entry_count;
       unsigned short version;
@@ -7704,22 +7703,18 @@ display_debug_ranges (struct dwarf_section *section,
 	  /* This section is 64-bit DWARF 3.  */
 	  SAFE_BYTE_GET_AND_INC (initial_length, start, 8, finish);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
+	offset_size = 4;
 
-      if (initial_length + initial_length_size > section->size)
+      if (initial_length > (size_t) (finish - start))
 	{
 	  /* If the length field has a relocation against it, then we should
 	     not complain if it is inaccurate (and probably negative).
 	     It is copied from .debug_line handling code.  */
 	  if (reloc_at (section, (start - section->start) - offset_size))
 	    {
-	      initial_length = (finish - start) - initial_length_size;
+	      initial_length = finish - start;
 	    }
 	  else
 	    {
@@ -7728,6 +7723,7 @@ display_debug_ranges (struct dwarf_section *section,
 	      return 0;
 	    }
 	}
+      finish = start + initial_length;
 
       /* Get and check the version number.  */
       SAFE_BYTE_GET_AND_INC (version, start, 2, finish);
@@ -7833,7 +7829,6 @@ display_debug_ranges (struct dwarf_section *section,
 
       pointer_size = (is_rnglists ? address_size : debug_info_p->pointer_size);
       offset = range_entry->ranges_offset;
-      next = section_begin + offset;
       base_address = debug_info_p->base_address;
 
       /* PR 17512: file: 001-101485-0.001:0.1.  */
@@ -7844,12 +7839,13 @@ display_debug_ranges (struct dwarf_section *section,
 	  continue;
 	}
 
-      if (next < section_begin || next >= finish)
+      if (offset > (size_t) (finish - section_begin))
 	{
 	  warn (_("Corrupt offset (%#8.8lx) in range entry %u\n"),
 		(unsigned long) offset, i);
 	  continue;
 	}
+      next = section_begin + offset;
 
       /* If multiple DWARF entities reference the same range then we will
          have multiple entries in the `range_entries' list for the same

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 15/19] read_cie
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (13 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 14/19] display_debug_ranges Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 16/19] display_debug_frames Alan Modra
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (read_cie): Add more sanity checks to ensure data
	pointer is not bumped past end.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 9243c853020..93e6d7319fa 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -8409,10 +8409,16 @@ read_cie (unsigned char *start, unsigned char *end,
     }
 
   if (strcmp (fc->augmentation, "eh") == 0)
-    start += eh_addr_size;
+    {
+      if (eh_addr_size > (size_t) (end - start))
+	goto fail;
+      start += eh_addr_size;
+    }
 
   if (version >= 4)
     {
+      if (2 > (size_t) (end - start))
+	goto fail;
       GET (fc->ptr_size, 1);
       if (fc->ptr_size < 1 || fc->ptr_size > 8)
 	{
@@ -8439,6 +8445,9 @@ read_cie (unsigned char *start, unsigned char *end,
   READ_ULEB (fc->code_factor, start, end);
   READ_SLEB (fc->data_factor, start, end);
 
+  if (start >= end)
+    goto fail;
+
   if (version == 1)
     {
       GET (fc->ra, 1);
@@ -8450,6 +8459,8 @@ read_cie (unsigned char *start, unsigned char *end,
 
   if (fc->augmentation[0] == 'z')
     {
+      if (start >= end)
+	goto fail;
       READ_ULEB (augmentation_data_len, start, end);
       augmentation_data = start;
       /* PR 17512: file: 11042-2589-0.004.  */

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 16/19] display_debug_frames
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (14 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 15/19] read_cie Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 17/19] display_debug_names Alan Modra
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_frames): Delete initial_length_size.
	Avoid pointer UB.  Constrain data reads to length given in header.
	Sanity check cie header length.  Only skip up to next FDE on
	finding augmentation data too long.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 93e6d7319fa..d2af05acb7c 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -8587,7 +8587,6 @@ display_debug_frames (struct dwarf_section *section,
       bfd_size_type augmentation_data_len = 0;
       unsigned int encoded_ptr_size = saved_eh_addr_size;
       unsigned int offset_size;
-      unsigned int initial_length_size;
       bool all_nops;
       static Frame_Chunk fde_fc;
 
@@ -8612,24 +8611,21 @@ display_debug_frames (struct dwarf_section *section,
 	{
 	  SAFE_BYTE_GET_AND_INC (length, start, 8, end);
 	  offset_size = 8;
-	  initial_length_size = 12;
 	}
       else
-	{
-	  offset_size = 4;
-	  initial_length_size = 4;
-	}
+	offset_size = 4;
 
-      block_end = saved_start + length + initial_length_size;
-      if (block_end > end || block_end < start)
+      if (length > (size_t) (end - start))
 	{
 	  warn ("Invalid length 0x%s in FDE at %#08lx\n",
 		dwarf_vmatoa_1 (NULL, length, offset_size),
 		(unsigned long) (saved_start - section_start));
 	  block_end = end;
 	}
+      else
+	block_end = start + length;
 
-      SAFE_BYTE_GET_AND_INC (cie_id, start, offset_size, end);
+      SAFE_BYTE_GET_AND_INC (cie_id, start, offset_size, block_end);
 
       if (is_eh ? (cie_id == 0) : ((offset_size == 4 && cie_id == DW_CIE_ID)
 				   || (offset_size == 8 && cie_id == DW64_CIE_ID)))
@@ -8637,7 +8633,7 @@ display_debug_frames (struct dwarf_section *section,
 	  int version;
 	  unsigned int mreg;
 
-	  start = read_cie (start, end, &cie, &version,
+	  start = read_cie (start, block_end, &cie, &version,
 			    &augmentation_data_len, &augmentation_data);
 	  /* PR 17512: file: 027-135133-0.005.  */
 	  if (cie == NULL)
@@ -8725,11 +8721,13 @@ display_debug_frames (struct dwarf_section *section,
 		      SAFE_BYTE_GET_AND_INC (length, cie_scan, 8, end);
 		      off_size = 8;
 		    }
-		  if (length != 0)
+		  if (length != 0 && length <= (size_t) (end - cie_scan))
 		    {
 		      dwarf_vma c_id;
+		      unsigned char *cie_end = cie_scan + length;
 
-		      SAFE_BYTE_GET_AND_INC (c_id, cie_scan, off_size, end);
+		      SAFE_BYTE_GET_AND_INC (c_id, cie_scan, off_size,
+					     cie_end);
 		      if (is_eh
 			  ? c_id == 0
 			  : ((off_size == 4 && c_id == DW_CIE_ID)
@@ -8738,7 +8736,7 @@ display_debug_frames (struct dwarf_section *section,
 			  int version;
 			  unsigned int mreg;
 
-			  read_cie (cie_scan, end, &cie, &version,
+			  read_cie (cie_scan, cie_end, &cie, &version,
 				    &augmentation_data_len, &augmentation_data);
 			  /* PR 17512: file: 3450-2098-0.004.  */
 			  if (cie == NULL)
@@ -8823,29 +8821,32 @@ display_debug_frames (struct dwarf_section *section,
 		  warn (_("Probably corrupt segment size: %d - using 4 instead\n"), fc->segment_size);
 		  fc->segment_size = 4;
 		}
-	      SAFE_BYTE_GET_AND_INC (segment_selector, start, fc->segment_size, end);
+	      SAFE_BYTE_GET_AND_INC (segment_selector, start,
+				     fc->segment_size, block_end);
 	    }
 
-	  fc->pc_begin = get_encoded_value (&start, fc->fde_encoding, section, end);
+	  fc->pc_begin = get_encoded_value (&start, fc->fde_encoding, section,
+					    block_end);
 
 	  /* FIXME: It appears that sometimes the final pc_range value is
 	     encoded in less than encoded_ptr_size bytes.  See the x86_64
 	     run of the "objcopy on compressed debug sections" test for an
 	     example of this.  */
-	  SAFE_BYTE_GET_AND_INC (fc->pc_range, start, encoded_ptr_size, end);
+	  SAFE_BYTE_GET_AND_INC (fc->pc_range, start, encoded_ptr_size,
+				 block_end);
 
 	  if (cie->augmentation[0] == 'z')
 	    {
-	      READ_ULEB (augmentation_data_len, start, end);
+	      READ_ULEB (augmentation_data_len, start, block_end);
 	      augmentation_data = start;
 	      /* PR 17512 file: 722-8446-0.004 and PR 22386.  */
-	      if (augmentation_data_len > (bfd_size_type) (end - start))
+	      if (augmentation_data_len > (bfd_size_type) (block_end - start))
 		{
 		  warn (_("Augmentation data too long: 0x%s, "
 			  "expected at most %#lx\n"),
 			dwarf_vmatoa ("x", augmentation_data_len),
-			(unsigned long) (end - start));
-		  start = end;
+			(unsigned long) (block_end - start));
+		  start = block_end;
 		  augmentation_data = NULL;
 		  augmentation_data_len = 0;
 		}
@@ -8889,7 +8890,6 @@ display_debug_frames (struct dwarf_section *section,
 	    {
 	      unsigned int reg, op, opa;
 	      unsigned long temp;
-	      unsigned char * new_start;
 
 	      op = *start++;
 	      opa = op & 0x3f;
@@ -8903,7 +8903,7 @@ display_debug_frames (struct dwarf_section *section,
 		case DW_CFA_advance_loc:
 		  break;
 		case DW_CFA_offset:
-		  SKIP_ULEB (start, end);
+		  SKIP_ULEB (start, block_end);
 		  if (frame_need_space (fc, opa) >= 0)
 		    fc->col_type[opa] = DW_CFA_undefined;
 		  break;
@@ -8912,105 +8912,111 @@ display_debug_frames (struct dwarf_section *section,
 		    fc->col_type[opa] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_set_loc:
-		  start += encoded_ptr_size;
+		  if ((size_t) (block_end - start) < encoded_ptr_size)
+		    start = block_end;
+		  else
+		    start += encoded_ptr_size;
 		  break;
 		case DW_CFA_advance_loc1:
-		  start += 1;
+		  if ((size_t) (block_end - start) < 1)
+		    start = block_end;
+		  else
+		    start += 1;
 		  break;
 		case DW_CFA_advance_loc2:
-		  start += 2;
+		  if ((size_t) (block_end - start) < 2)
+		    start = block_end;
+		  else
+		    start += 2;
 		  break;
 		case DW_CFA_advance_loc4:
-		  start += 4;
+		  if ((size_t) (block_end - start) < 4)
+		    start = block_end;
+		  else
+		    start += 4;
 		  break;
 		case DW_CFA_offset_extended:
 		case DW_CFA_val_offset:
-		  READ_ULEB (reg, start, end);
-		  SKIP_ULEB (start, end);
+		  READ_ULEB (reg, start, block_end);
+		  SKIP_ULEB (start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_restore_extended:
-		  READ_ULEB (reg, start, end);
+		  READ_ULEB (reg, start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_undefined:
-		  READ_ULEB (reg, start, end);
+		  READ_ULEB (reg, start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_same_value:
-		  READ_ULEB (reg, start, end);
+		  READ_ULEB (reg, start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_register:
-		  READ_ULEB (reg, start, end);
-		  SKIP_ULEB (start, end);
+		  READ_ULEB (reg, start, block_end);
+		  SKIP_ULEB (start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_def_cfa:
-		  SKIP_ULEB (start, end);
-		  SKIP_ULEB (start, end);
+		  SKIP_ULEB (start, block_end);
+		  SKIP_ULEB (start, block_end);
 		  break;
 		case DW_CFA_def_cfa_register:
-		  SKIP_ULEB (start, end);
+		  SKIP_ULEB (start, block_end);
 		  break;
 		case DW_CFA_def_cfa_offset:
-		  SKIP_ULEB (start, end);
+		  SKIP_ULEB (start, block_end);
 		  break;
 		case DW_CFA_def_cfa_expression:
-		  READ_ULEB (temp, start, end);
-		  new_start = start + temp;
-		  if (new_start < start)
-		    {
-		      warn (_("Corrupt CFA_def expression value: %lu\n"), temp);
-		      start = block_end;
-		    }
+		  READ_ULEB (temp, start, block_end);
+		  if ((size_t) (block_end - start) < temp)
+		    start = block_end;
 		  else
-		    start = new_start;
+		    start += temp;
 		  break;
 		case DW_CFA_expression:
 		case DW_CFA_val_expression:
-		  READ_ULEB (reg, start, end);
-		  READ_ULEB (temp, start, end);
-		  new_start = start + temp;
-		  if (new_start < start)
-		    {
-		      /* PR 17512: file:306-192417-0.005.  */
-		      warn (_("Corrupt CFA expression value: %lu\n"), temp);
-		      start = block_end;
-		    }
+		  READ_ULEB (reg, start, block_end);
+		  READ_ULEB (temp, start, block_end);
+		  if ((size_t) (block_end - start) < temp)
+		    start = block_end;
 		  else
-		    start = new_start;
+		    start += temp;
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_offset_extended_sf:
 		case DW_CFA_val_offset_sf:
-		  READ_ULEB (reg, start, end);
-		  SKIP_SLEB (start, end);
+		  READ_ULEB (reg, start, block_end);
+		  SKIP_SLEB (start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_def_cfa_sf:
-		  SKIP_ULEB (start, end);
-		  SKIP_SLEB (start, end);
+		  SKIP_ULEB (start, block_end);
+		  SKIP_SLEB (start, block_end);
 		  break;
 		case DW_CFA_def_cfa_offset_sf:
-		  SKIP_SLEB (start, end);
+		  SKIP_SLEB (start, block_end);
 		  break;
 		case DW_CFA_MIPS_advance_loc8:
-		  start += 8;
+		  if ((size_t) (block_end - start) < 8)
+		    start = block_end;
+		  else
+		    start += 8;
 		  break;
 		case DW_CFA_GNU_args_size:
-		  SKIP_ULEB (start, end);
+		  SKIP_ULEB (start, block_end);
 		  break;
 		case DW_CFA_GNU_negative_offset_extended:
-		  READ_ULEB (reg, start, end);
-		  SKIP_ULEB (start, end);
+		  READ_ULEB (reg, start, block_end);
+		  SKIP_ULEB (start, block_end);
 		  if (frame_need_space (fc, reg) >= 0)
 		    fc->col_type[reg] = DW_CFA_undefined;
 		  break;
@@ -9028,7 +9034,6 @@ display_debug_frames (struct dwarf_section *section,
 
       while (start < block_end)
 	{
-	  unsigned char * tmp;
 	  unsigned op, opa;
 	  unsigned long ul, roffs;
 	  /* Note: It is tempting to use an unsigned long for 'reg' but there
@@ -9066,7 +9071,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_offset:
-	      READ_ULEB (roffs, start, end);
+	      READ_ULEB (roffs, start, block_end);
 	      if (opa >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9104,7 +9109,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_set_loc:
-	      vma = get_encoded_value (&start, fc->fde_encoding, section, block_end);
+	      vma = get_encoded_value (&start, fc->fde_encoding, section,
+				       block_end);
 	      if (do_debug_frames_interp)
 		frame_display_row (fc, &need_col_headers, &max_regs);
 	      else
@@ -9114,7 +9120,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_advance_loc1:
-	      SAFE_BYTE_GET_AND_INC (ofs, start, 1, end);
+	      SAFE_BYTE_GET_AND_INC (ofs, start, 1, block_end);
 	      if (do_debug_frames_interp)
 		frame_display_row (fc, &need_col_headers, &max_regs);
 	      else
@@ -9153,8 +9159,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_offset_extended:
-	      READ_ULEB (reg, start, end);
-	      READ_ULEB (roffs, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_ULEB (roffs, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9169,8 +9175,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_val_offset:
-	      READ_ULEB (reg, start, end);
-	      READ_ULEB (roffs, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_ULEB (roffs, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9185,7 +9191,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_restore_extended:
-	      READ_ULEB (reg, start, end);
+	      READ_ULEB (reg, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9207,7 +9213,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_undefined:
-	      READ_ULEB (reg, start, end);
+	      READ_ULEB (reg, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9221,7 +9227,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_same_value:
-	      READ_ULEB (reg, start, end);
+	      READ_ULEB (reg, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9235,8 +9241,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_register:
-	      READ_ULEB (reg, start, end);
-	      READ_ULEB (roffs, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_ULEB (roffs, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9299,8 +9305,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa:
-	      READ_ULEB (fc->cfa_reg, start, end);
-	      READ_ULEB (fc->cfa_offset, start, end);
+	      READ_ULEB (fc->cfa_reg, start, block_end);
+	      READ_ULEB (fc->cfa_offset, start, block_end);
 	      fc->cfa_exp = 0;
 	      if (! do_debug_frames_interp)
 		printf ("  DW_CFA_def_cfa: %s ofs %d\n",
@@ -9308,7 +9314,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa_register:
-	      READ_ULEB (fc->cfa_reg, start, end);
+	      READ_ULEB (fc->cfa_reg, start, block_end);
 	      fc->cfa_exp = 0;
 	      if (! do_debug_frames_interp)
 		printf ("  DW_CFA_def_cfa_register: %s\n",
@@ -9316,7 +9322,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa_offset:
-	      READ_ULEB (fc->cfa_offset, start, end);
+	      READ_ULEB (fc->cfa_offset, start, block_end);
 	      if (! do_debug_frames_interp)
 		printf ("  DW_CFA_def_cfa_offset: %d\n", (int) fc->cfa_offset);
 	      break;
@@ -9327,8 +9333,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa_expression:
-	      READ_ULEB (ul, start, end);
-	      if (start >= block_end || ul > (unsigned long) (block_end - start))
+	      READ_ULEB (ul, start, block_end);
+	      if (ul > (size_t) (block_end - start))
 		{
 		  printf (_("  DW_CFA_def_cfa_expression: <corrupt len %lu>\n"), ul);
 		  break;
@@ -9345,14 +9351,13 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_expression:
-	      READ_ULEB (reg, start, end);
-	      READ_ULEB (ul, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_ULEB (ul, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
 	      /* PR 17512: file: 069-133014-0.006.  */
 	      /* PR 17512: file: 98c02eb4.  */
-	      tmp = start + ul;
-	      if (start >= block_end || tmp > block_end || tmp < start)
+	      if (ul > (size_t) (block_end - start))
 		{
 		  printf (_("  DW_CFA_expression: <corrupt len %lu>\n"), ul);
 		  break;
@@ -9367,16 +9372,15 @@ display_debug_frames (struct dwarf_section *section,
 		}
 	      if (*reg_prefix == '\0')
 		fc->col_type[reg] = DW_CFA_expression;
-	      start = tmp;
+	      start += ul;
 	      break;
 
 	    case DW_CFA_val_expression:
-	      READ_ULEB (reg, start, end);
-	      READ_ULEB (ul, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_ULEB (ul, start, block_end);
 	      if (reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
-	      tmp = start + ul;
-	      if (start >= block_end || tmp > block_end || tmp < start)
+	      if (ul > (size_t) (block_end - start))
 		{
 		  printf ("  DW_CFA_val_expression: <corrupt len %lu>\n", ul);
 		  break;
@@ -9391,12 +9395,12 @@ display_debug_frames (struct dwarf_section *section,
 		}
 	      if (*reg_prefix == '\0')
 		fc->col_type[reg] = DW_CFA_val_expression;
-	      start = tmp;
+	      start += ul;
 	      break;
 
 	    case DW_CFA_offset_extended_sf:
-	      READ_ULEB (reg, start, end);
-	      READ_SLEB (l, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_SLEB (l, start, block_end);
 	      if (frame_need_space (fc, reg) < 0)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9411,8 +9415,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_val_offset_sf:
-	      READ_ULEB (reg, start, end);
-	      READ_SLEB (l, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_SLEB (l, start, block_end);
 	      if (frame_need_space (fc, reg) < 0)
 		reg_prefix = bad_reg;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
@@ -9427,8 +9431,8 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa_sf:
-	      READ_ULEB (fc->cfa_reg, start, end);
-	      READ_ULEB (fc->cfa_offset, start, end);
+	      READ_ULEB (fc->cfa_reg, start, block_end);
+	      READ_ULEB (fc->cfa_offset, start, block_end);
 	      fc->cfa_offset = fc->cfa_offset * fc->data_factor;
 	      fc->cfa_exp = 0;
 	      if (! do_debug_frames_interp)
@@ -9437,7 +9441,7 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_def_cfa_offset_sf:
-	      READ_ULEB (fc->cfa_offset, start, end);
+	      READ_ULEB (fc->cfa_offset, start, block_end);
 	      fc->cfa_offset *= fc->data_factor;
 	      if (! do_debug_frames_interp)
 		printf ("  DW_CFA_def_cfa_offset_sf: %d\n", (int) fc->cfa_offset);
@@ -9462,14 +9466,14 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_GNU_args_size:
-	      READ_ULEB (ul, start, end);
+	      READ_ULEB (ul, start, block_end);
 	      if (! do_debug_frames_interp)
 		printf ("  DW_CFA_GNU_args_size: %ld\n", ul);
 	      break;
 
 	    case DW_CFA_GNU_negative_offset_extended:
-	      READ_ULEB (reg, start, end);
-	      READ_SLEB (l, start, end);
+	      READ_ULEB (reg, start, block_end);
+	      READ_SLEB (l, start, block_end);
 	      l = - l;
 	      if (frame_need_space (fc, reg) < 0)
 		reg_prefix = bad_reg;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 17/19] display_debug_names
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (15 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 16/19] display_debug_frames Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 18/19] display_gdb_index Alan Modra
  2021-05-15  8:09 ` [PATCH 19/19] process_cu_tu_index Alan Modra
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_debug_names): Complain when header length is
	too small.  Avoid pointer UB.  Sanity check augmentation string,
	CU table, TU table and foreign TU table sizes.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index d2af05acb7c..d06dd4bbbf9 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -9571,12 +9571,12 @@ display_debug_names (struct dwarf_section *section, void *file)
       unsigned int offset_size;
       uint16_t dwarf_version, padding;
       uint32_t comp_unit_count, local_type_unit_count, foreign_type_unit_count;
-      uint32_t bucket_count, name_count, abbrev_table_size;
+      uint64_t bucket_count, name_count, abbrev_table_size;
       uint32_t augmentation_string_size;
       unsigned int i;
-      unsigned long sec_off;
       bool augmentation_printable;
       const char *augmentation_string;
+      size_t total;
 
       unit_start = hdrptr;
 
@@ -9591,18 +9591,18 @@ display_debug_names (struct dwarf_section *section, void *file)
 	}
       else
 	offset_size = 4;
-      unit_end = hdrptr + unit_length;
 
-      sec_off = hdrptr - section->start;
-      if (sec_off + unit_length < sec_off
-	  || sec_off + unit_length > section->size)
+      if (unit_length > (size_t) (section_end - hdrptr)
+	  || unit_length < 2 + 2 + 4 * 7)
 	{
+	too_short:
 	  warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
 		section->name,
 		(unsigned long) (unit_start - section->start),
 		dwarf_vmatoa ("x", unit_length));
 	  return 0;
 	}
+      unit_end = hdrptr + unit_length;
 
       /* Get and check the version number.  */
       SAFE_BYTE_GET_AND_INC (dwarf_version, hdrptr, 2, unit_end);
@@ -9640,6 +9640,8 @@ display_debug_names (struct dwarf_section *section, void *file)
 		augmentation_string_size);
 	  augmentation_string_size += (-augmentation_string_size) & 3;
 	}
+      if (augmentation_string_size > (size_t) (unit_end - hdrptr))
+	goto too_short;
 
       printf (_("Augmentation string:"));
 
@@ -9669,6 +9671,9 @@ display_debug_names (struct dwarf_section *section, void *file)
       putchar ('\n');
 
       printf (_("CU table:\n"));
+      if (_mul_overflow (comp_unit_count, offset_size, &total)
+	  || total > (size_t) (unit_end - hdrptr))
+	goto too_short;
       for (i = 0; i < comp_unit_count; i++)
 	{
 	  uint64_t cu_offset;
@@ -9679,6 +9684,9 @@ display_debug_names (struct dwarf_section *section, void *file)
       putchar ('\n');
 
       printf (_("TU table:\n"));
+      if (_mul_overflow (local_type_unit_count, offset_size, &total)
+	  || total > (size_t) (unit_end - hdrptr))
+	goto too_short;
       for (i = 0; i < local_type_unit_count; i++)
 	{
 	  uint64_t tu_offset;
@@ -9689,6 +9697,9 @@ display_debug_names (struct dwarf_section *section, void *file)
       putchar ('\n');
 
       printf (_("Foreign TU table:\n"));
+      if (_mul_overflow (foreign_type_unit_count, 8, &total)
+	  || total > (size_t) (unit_end - hdrptr))
+	goto too_short;
       for (i = 0; i < foreign_type_unit_count; i++)
 	{
 	  uint64_t signature;
@@ -9700,6 +9711,18 @@ display_debug_names (struct dwarf_section *section, void *file)
 	}
       putchar ('\n');
 
+      uint64_t xtra = (bucket_count * sizeof (uint32_t)
+		       + name_count * (sizeof (uint32_t) + 2 * offset_size)
+		       + abbrev_table_size);
+      if (xtra > (size_t) (unit_end - hdrptr))
+	{
+	  warn (_("Entry pool offset (0x%lx) exceeds unit size 0x%lx "
+		  "for unit 0x%lx in the debug_names\n"),
+		(long) xtra,
+		(long) (unit_end - unit_start),
+		(long) (unit_start - section->start));
+	  return 0;
+	}
       const uint32_t *const hash_table_buckets = (uint32_t *) hdrptr;
       hdrptr += bucket_count * sizeof (uint32_t);
       const uint32_t *const hash_table_hashes = (uint32_t *) hdrptr;
@@ -9712,15 +9735,6 @@ display_debug_names (struct dwarf_section *section, void *file)
       hdrptr += abbrev_table_size;
       const unsigned char *const abbrev_table_end = hdrptr;
       unsigned char *const entry_pool = hdrptr;
-      if (hdrptr > unit_end)
-	{
-	  warn (_("Entry pool offset (0x%lx) exceeds unit size 0x%lx "
-		  "for unit 0x%lx in the debug_names\n"),
-		(long) (hdrptr - section->start),
-		(long) (unit_end - section->start),
-		(long) (unit_start - section->start));
-	  return 0;
-	}
 
       size_t buckets_filled = 0;
       size_t bucketi;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 18/19] display_gdb_index
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (16 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 17/19] display_debug_names Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  2021-05-15  8:09 ` [PATCH 19/19] process_cu_tu_index Alan Modra
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (display_gdb_index): Avoid pointer UB and overflow in
	length calculations.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index d06dd4bbbf9..db02be723b4 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -10105,7 +10105,7 @@ display_gdb_index (struct dwarf_section *section,
   symbol_table = start + symbol_table_offset;
   constant_pool = start + constant_pool_offset;
 
-  if (address_table + address_table_size > section->start + section->size)
+  if (address_table_offset + address_table_size > section->size)
     {
       warn (_("Address table extends beyond end of section.\n"));
       return 0;
@@ -10160,11 +10160,9 @@ display_gdb_index (struct dwarf_section *section,
 	  || cu_vector_offset != 0)
 	{
 	  unsigned int j;
-	  unsigned char * adr;
 
-	  adr = constant_pool + name_offset;
 	  /* PR 17531: file: 5b7b07ad.  */
-	  if (adr < constant_pool || adr >= section->start + section->size)
+	  if (name_offset >= section->size - constant_pool_offset)
 	    {
 	      printf (_("[%3u] <corrupt offset: %x>"), i, name_offset);
 	      warn (_("Corrupt name offset of 0x%x found for symbol table slot %d\n"),
@@ -10175,8 +10173,8 @@ display_gdb_index (struct dwarf_section *section,
 		    (int) (section->size - (constant_pool_offset + name_offset)),
 		    constant_pool + name_offset);
 
-	  adr = constant_pool + cu_vector_offset;
-	  if (adr < constant_pool || adr >= section->start + section->size - 3)
+	  if (section->size - constant_pool_offset < 4
+	      || cu_vector_offset > section->size - constant_pool_offset - 4)
 	    {
 	      printf (_("<invalid CU vector offset: %x>\n"), cu_vector_offset);
 	      warn (_("Corrupt CU vector offset of 0x%x found for symbol table slot %d\n"),
@@ -10184,12 +10182,10 @@ display_gdb_index (struct dwarf_section *section,
 	      continue;
 	    }
 
-	  num_cus = byte_get_little_endian (adr, 4);
+	  num_cus = byte_get_little_endian (constant_pool + cu_vector_offset, 4);
 
-	  adr = constant_pool + cu_vector_offset + 4 + num_cus * 4;
-	  if (num_cus * 4 < num_cus
-	      || adr >= section->start + section->size
-	      || adr < constant_pool)
+	  if ((uint64_t) num_cus * 4 > section->size - (constant_pool_offset
+							+ cu_vector_offset + 4))
 	    {
 	      printf ("<invalid number of CUs: %d>\n", num_cus);
 	      warn (_("Invalid number of CUs (0x%x) for symbol table slot %d\n"),

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 19/19] process_cu_tu_index
  2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
                   ` (17 preceding siblings ...)
  2021-05-15  8:09 ` [PATCH 18/19] display_gdb_index Alan Modra
@ 2021-05-15  8:09 ` Alan Modra
  18 siblings, 0 replies; 20+ messages in thread
From: Alan Modra @ 2021-05-15  8:09 UTC (permalink / raw)
  To: binutils

	* dwarf.c (process_cu_tu_index): Avoid pointer UB.  Use _mul_overflow.
	Delete dead code.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index db02be723b4..beac2260768 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -10320,6 +10320,7 @@ process_cu_tu_index (struct dwarf_section *section, int do_display)
   unsigned int i;
   unsigned int j;
   dwarf_vma signature;
+  size_t total;
 
   /* PR 17512: file: 002-168123-0.004.  */
   if (phdr == NULL)
@@ -10357,10 +10358,8 @@ process_cu_tu_index (struct dwarf_section *section, int do_display)
     }
 
   /* PR 17531: file: 45d69832.  */
-  if ((size_t) nslots * 8 / 8 != nslots
-      || phash < phdr || phash > limit
-      || pindex < phash || pindex > limit
-      || ppool < pindex || ppool > limit)
+  if (_mul_overflow ((size_t) nslots, 12, &total)
+      || total > (size_t) (limit - phash))
     {
       warn (ngettext ("Section %s is too small for %u slot\n",
 		      "Section %s is too small for %u slots\n",
@@ -10427,23 +10426,21 @@ process_cu_tu_index (struct dwarf_section *section, int do_display)
       unsigned char *pi = pindex;
       unsigned char *poffsets = ppool + (size_t) ncols * 4;
       unsigned char *psizes = poffsets + (size_t) nused * ncols * 4;
-      unsigned char *pend = psizes + (size_t) nused * ncols * 4;
       bool is_tu_index;
       struct cu_tu_set *this_set = NULL;
       unsigned int row;
       unsigned char *prow;
+      size_t temp;
 
       is_tu_index = strcmp (section->name, ".debug_tu_index") == 0;
 
       /* PR 17531: file: 0dd159bf.
 	 Check for integer overflow (can occur when size_t is 32-bit)
 	 with overlarge ncols or nused values.  */
-      if (ncols > 0
-	  && ((size_t) ncols * 4 / 4 != ncols
-	      || (size_t) nused * ncols * 4 / ((size_t) ncols * 4) != nused
-	      || poffsets < ppool || poffsets > limit
-	      || psizes < poffsets || psizes > limit
-	      || pend < psizes || pend > limit))
+      if (nused == -1u
+	  || _mul_overflow ((size_t) ncols, 4, &temp)
+	  || _mul_overflow ((size_t) nused + 1, temp, &total)
+	  || total > (size_t) (limit - ppool))
 	{
 	  warn (_("Section %s too small for offset and size tables\n"),
 		section->name);
@@ -10502,25 +10499,10 @@ process_cu_tu_index (struct dwarf_section *section, int do_display)
 		{
 		  size_t num_copy = sizeof (uint64_t);
 
-		  /* PR 23064: Beware of buffer overflow.  */
-		  if (ph + num_copy < limit)
-		    memcpy (&this_set[row - 1].signature, ph, num_copy);
-		  else
-		    {
-		      warn (_("Signature (%p) extends beyond end of space in section\n"), ph);
-		      return 0;
-		    }
+		  memcpy (&this_set[row - 1].signature, ph, num_copy);
 		}
 
 	      prow = poffsets + (row - 1) * ncols * 4;
-	      /* PR 17531: file: b8ce60a8.  */
-	      if (prow < poffsets || prow > limit)
-		{
-		  warn (_("Row index (%u) * num columns (%u) > space remaining in section\n"),
-			row, ncols);
-		  return 0;
-		}
-
 	      if (do_display)
 		printf (_("  [%3d] 0x%s"),
 			i, dwarf_vmatoa ("x", signature));

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-05-15  8:11 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15  8:09 [PATCH 00/19] Pointer UB in binutils/dwarf.c Alan Modra
2021-05-15  8:09 ` [PATCH 01/19] _mul_overflow and get_encoded_value Alan Modra
2021-05-15  8:09 ` [PATCH 02/19] SAFE_BYTE_GET_INTERNAL Alan Modra
2021-05-15  8:09 ` [PATCH 03/19] process_debug_info Alan Modra
2021-05-15  8:09 ` [PATCH 04/19] read_debug_line_header Alan Modra
2021-05-15  8:09 ` [PATCH 05/19] display_debug_lines_decoded Alan Modra
2021-05-15  8:09 ` [PATCH 06/19] display_debug_pubnames_worker Alan Modra
2021-05-15  8:09 ` [PATCH 07/19] display_debug_macinfo Alan Modra
2021-05-15  8:09 ` [PATCH 08/19] get_line_filename_and_dirname Alan Modra
2021-05-15  8:09 ` [PATCH 09/19] display_debug_macro Alan Modra
2021-05-15  8:09 ` [PATCH 10/19] display_loc_list Alan Modra
2021-05-15  8:09 ` [PATCH 11/19] display_debug_aranges Alan Modra
2021-05-15  8:09 ` [PATCH 12/19] display_debug_str_offsets Alan Modra
2021-05-15  8:09 ` [PATCH 13/19] display_debug_rnglists_list Alan Modra
2021-05-15  8:09 ` [PATCH 14/19] display_debug_ranges Alan Modra
2021-05-15  8:09 ` [PATCH 15/19] read_cie Alan Modra
2021-05-15  8:09 ` [PATCH 16/19] display_debug_frames Alan Modra
2021-05-15  8:09 ` [PATCH 17/19] display_debug_names Alan Modra
2021-05-15  8:09 ` [PATCH 18/19] display_gdb_index Alan Modra
2021-05-15  8:09 ` [PATCH 19/19] process_cu_tu_index Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).