DWARF header checks

This patch tidies DWARF header checks, consolidating the "negative"
checks (which are really overflow checks) with the section size
check.  In a number of cases this also ensures that small negative
lengths are caught.  For instance

      hdrptr = start + arange.ar_length + initial_length_size;
      if (hdrptr < start || hdrptr > end)

does not detect ar_length in the range [-initial_length_size,-1].

	* dwarf.c (process_debug_info): Consolidate header length checks.
	(display_debug_pubnames_worker): Use "start" to read header.
	Properly check header length and report errors earlier.
	Simplify loop printing pubnames.
	(get_line_filename_and_dirname): Catch small negative "length"
	values.
	(display_debug_aranges): Likewise.  Report header errors
	earlier using standardized message.
	(display_debug_names): Likewise.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 4fb4eb4..ca4990e 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,15 @@
+2017-10-06  Alan Modra  <amodra@gmail.com>
+
+       * dwarf.c (process_debug_info): Consolidate header length checks.
+       (display_debug_pubnames_worker): Use "start" to read header.
+       Properly check header length and report errors earlier.
+       Simplify loop printing pubnames.
+       (get_line_filename_and_dirname): Catch small negative "length"
+       values.
+       (display_debug_aranges): Likewise.  Report header errors
+       earlier using standardized message.
+       (display_debug_names): Likewise.
+
 2017-10-05  Joseph Myers  <joseph@codesourcery.com>
 
        * readelf.c (decode_arm_unwind): Initialize res to TRUE.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index f8c726c..91f95ff 100644 (file)
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -2590,6 +2590,7 @@ 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_high = 0;
@@ -2729,20 +2730,13 @@ process_debug_info (struct dwarf_section *section,
            }
        }
 
-      if (cu_offset + compunit.cu_length + initial_length_size
-         > section->size)
+      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, length of CU at %s"
-                 " extends beyond end of section (length = %s)\n"),
-               dwarf_vmatoa ("x", cu_offset),
-               dwarf_vmatoa ("x", compunit.cu_length));
-         num_units = unit;
-         break;
-       }
-      else if (compunit.cu_length + initial_length_size < initial_length_size)
-       {
-         warn (_("Debug info is corrupted, length of CU at %s is negative (%s)\n"),
-               dwarf_vmatoa ("x", cu_offset),
+         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;
@@ -2751,13 +2745,6 @@ process_debug_info (struct dwarf_section *section,
       tags = hdrptr;
       start += compunit.cu_length + initial_length_size;
 
-      if (start > end)
-       {
-         warn (_("Debug info is corrupt.  CU at %s extends beyond end of section"),
-               dwarf_vmatoa ("x", cu_offset));
-         start = end;
-       }
-
       if (compunit.cu_version < 2 || compunit.cu_version > 5)
        {
          warn (_("CU at offset %s contains corrupt or "
@@ -4489,16 +4476,13 @@ display_debug_pubnames_worker (struct dwarf_section *section,
   while (start < end)
     {
       unsigned char *data;
-      unsigned char *adr;
-      dwarf_vma offset;
+      unsigned long sec_off;
       unsigned int offset_size, initial_length_size;
 
-      data = start;
-
-      SAFE_BYTE_GET_AND_INC (names.pn_length, data, 4, end);
+      SAFE_BYTE_GET_AND_INC (names.pn_length, start, 4, end);
       if (names.pn_length == 0xffffffff)
        {
-         SAFE_BYTE_GET_AND_INC (names.pn_length, data, 8, end);
+         SAFE_BYTE_GET_AND_INC (names.pn_length, start, 8, end);
          offset_size = 8;
          initial_length_size = 12;
        }
@@ -4508,6 +4492,20 @@ display_debug_pubnames_worker (struct dwarf_section *section,
          initial_length_size = 4;
        }
 
+      sec_off = start - section->start;
+      if (sec_off + names.pn_length < sec_off
+         || sec_off + names.pn_length > section->size)
+       {
+         warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
+               section->name,
+               sec_off - initial_length_size,
+               dwarf_vmatoa ("x", names.pn_length));
+         break;
+       }
+
+      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);
 
@@ -4519,18 +4517,6 @@ display_debug_pubnames_worker (struct dwarf_section *section,
 
       SAFE_BYTE_GET_AND_INC (names.pn_size, data, offset_size, end);
 
-      adr = start + names.pn_length + initial_length_size;
-      /* PR 17531: file: 7615b6b2.  */
-      if ((dwarf_signed_vma) names.pn_length < 0
-         /* PR 17531: file: a5dbeaa7. */
-         || adr < start)
-       {
-         warn (_("Negative length for public name: 0x%lx\n"), (long) names.pn_length);
-         start = end;
-       }
-      else
-       start = adr;
-
       printf (_("  Length:                              %ld\n"),
              (long) names.pn_length);
       printf (_("  Version:                             %d\n"),
@@ -4558,51 +4544,51 @@ display_debug_pubnames_worker (struct dwarf_section *section,
       else
        printf (_("\n    Offset\tName\n"));
 
-      do
+      while (1)
        {
          bfd_size_type maxprint;
+         dwarf_vma offset;
 
          SAFE_BYTE_GET (offset, data, offset_size, end);
 
-         if (offset != 0)
-           {
-             data += offset_size;
-             if (data >= end)
-               break;
-             maxprint = (end - data) - 1;
+         if (offset == 0)
+           break;
 
-             if (is_gnu)
-               {
-                 unsigned int kind_data;
-                 gdb_index_symbol_kind kind;
-                 const char *kind_name;
-                 int is_static;
-
-                 SAFE_BYTE_GET (kind_data, data, 1, end);
-                 data++;
-                 maxprint --;
-                 /* GCC computes the kind as the upper byte in the CU index
-                    word, and then right shifts it by the CU index size.
-                    Left shift KIND to where the gdb-index.h accessor macros
-                    can use it.  */
-                 kind_data <<= GDB_INDEX_CU_BITSIZE;
-                 kind = GDB_INDEX_SYMBOL_KIND_VALUE (kind_data);
-                 kind_name = get_gdb_index_symbol_kind_name (kind);
-                 is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (kind_data);
-                 printf ("    %-6lx  %s,%-10s  %.*s\n",
-                         (unsigned long) offset, is_static ? _("s") : _("g"),
-                         kind_name, (int) maxprint, data);
-               }
-             else
-               printf ("    %-6lx\t%.*s\n",
-                       (unsigned long) offset, (int) maxprint, data);
+         data += offset_size;
+         if (data >= end)
+           break;
+         maxprint = (end - data) - 1;
 
-             data += strnlen ((char *) data, maxprint) + 1;
-             if (data >= end)
-               break;
+         if (is_gnu)
+           {
+             unsigned int kind_data;
+             gdb_index_symbol_kind kind;
+             const char *kind_name;
+             int is_static;
+
+             SAFE_BYTE_GET (kind_data, data, 1, end);
+             data++;
+             maxprint --;
+             /* GCC computes the kind as the upper byte in the CU index
+                word, and then right shifts it by the CU index size.
+                Left shift KIND to where the gdb-index.h accessor macros
+                can use it.  */
+             kind_data <<= GDB_INDEX_CU_BITSIZE;
+             kind = GDB_INDEX_SYMBOL_KIND_VALUE (kind_data);
+             kind_name = get_gdb_index_symbol_kind_name (kind);
+             is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (kind_data);
+             printf ("    %-6lx  %s,%-10s  %.*s\n",
+                     (unsigned long) offset, is_static ? _("s") : _("g"),
+                     kind_name, (int) maxprint, data);
            }
+         else
+           printf ("    %-6lx\t%.*s\n",
+                   (unsigned long) offset, (int) maxprint, data);
+
+         data += strnlen ((char *) data, maxprint) + 1;
+         if (data >= end)
+           break;
        }
-      while (offset != 0);
     }
 
   printf ("\n");
@@ -4735,7 +4721,8 @@ get_line_filename_and_dirname (dwarf_vma line_offset,
       offset_size = 4;
       initial_length_size = 4;
     }
-  if (length + initial_length_size > section->size)
+  if (length + initial_length_size < length
+      || length + initial_length_size > section->size)
     return NULL;
 
   SAFE_BYTE_GET_AND_INC (version, hdrptr, 2, end);
@@ -6027,6 +6014,7 @@ display_debug_aranges (struct dwarf_section *section,
       unsigned char *addr_ranges;
       dwarf_vma length;
       dwarf_vma address;
+      unsigned long sec_off;
       unsigned char address_size;
       int excess;
       unsigned int offset_size;
@@ -6047,6 +6035,17 @@ display_debug_aranges (struct dwarf_section *section,
          initial_length_size = 4;
        }
 
+      sec_off = hdrptr - section->start;
+      if (sec_off + arange.ar_length < sec_off
+         || sec_off + arange.ar_length > section->size)
+       {
+         warn (_("Debug info is corrupted, %s header at %#lx has length %s\n"),
+               section->name,
+               sec_off - initial_length_size,
+               dwarf_vmatoa ("x", arange.ar_length));
+         break;
+       }
+
       SAFE_BYTE_GET_AND_INC (arange.ar_version, hdrptr, 2, end);
       SAFE_BYTE_GET_AND_INC (arange.ar_info_offset, hdrptr, offset_size, end);
 
@@ -6109,13 +6108,7 @@ display_debug_aranges (struct dwarf_section *section,
       if (excess)
        addr_ranges += (2 * address_size) - excess;
 
-      hdrptr = start + arange.ar_length + initial_length_size;
-      if (hdrptr < start || hdrptr > end)
-       {
-         error (_("Excessive header length: %lx\n"), (long) arange.ar_length);
-         break;
-       }
-      start = hdrptr;
+      start += arange.ar_length + initial_length_size;
 
       while (addr_ranges + 2 * address_size <= start)
        {
@@ -8030,6 +8023,7 @@ display_debug_names (struct dwarf_section *section, void *file)
       uint32_t bucket_count, name_count, abbrev_table_size;
       uint32_t augmentation_string_size;
       unsigned int i;
+      unsigned long sec_off;
 
       unit_start = hdrptr;
 
@@ -8046,11 +8040,14 @@ display_debug_names (struct dwarf_section *section, void *file)
        offset_size = 4;
       unit_end = hdrptr + unit_length;
 
-      if ((hdrptr - section->start) + unit_length > section->size)
+      sec_off = hdrptr - section->start;
+      if (sec_off + unit_length < sec_off
+         || sec_off + unit_length > section->size)
        {
-         warn (_("The length field (0x%lx) for unit 0x%lx in the debug_names "
-                 "header is wrong - the section is too small\n"),
-               (long) unit_length, (long) (unit_start - section->start));
+         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;
        }