PR21990, Integer overflow in process_version_sections

This tidies some of the overflow checking when processing verneed
and verdef sections.

	PR 21990
	* readelf.c (process_version_sections <SHT_GNU_verneed>): Check
	for invalid vn_next field before adding to idx.  Use unsigned
	long for index vars.  Move index checks.
	<SHT_GNU_verdef>: Likewise for vd_next.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 3467f09d9f0031ba619b9d7a5fa6e62da94d61c3..4bae660c241933359dd90c898ec20bf75343326d 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,11 @@
+2017-08-23  Alan Modra  <amodra@gmail.com>
+
+       PR 21990
+       * readelf.c (process_version_sections <SHT_GNU_verneed>): Check
+       for invalid vn_next field before adding to idx.  Use unsigned
+       long for index vars.  Move index checks.
+       <SHT_GNU_verdef>: Likewise for vd_next.
+
 2017-08-17  Nick Clifton  <nickc@redhat.com>
 
        * testsuite/binutils-all/note-3-64.s: New test.  Checks assembly

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 1992126bbba775d44140181a959ee5fe2b8d4566..07cd2b01c9dcb9952409be7592c7aeef9506e83c 100644 (file)
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -10153,9 +10153,9 @@ process_version_sections (FILE * file)
        case SHT_GNU_verdef:
          {
            Elf_External_Verdef * edefs;
-           unsigned int idx;
-           unsigned int cnt;
-           unsigned int end;
+           unsigned long idx;
+           unsigned long cnt;
+           unsigned long end;
            char * endbuf;
 
            found = TRUE;
@@ -10187,13 +10187,9 @@ process_version_sections (FILE * file)
                Elf_Internal_Verdef ent;
                Elf_External_Verdaux * eaux;
                Elf_Internal_Verdaux aux;
-               unsigned int isum;
+               unsigned long isum;
                int j;
 
-               /* Check for very large indices.  */
-               if (idx > (size_t) (endbuf - (char *) edefs))
-                 break;
-
                vstart = ((char *) edefs) + idx;
                if (vstart + sizeof (*edef) > endbuf)
                  break;
@@ -10208,15 +10204,16 @@ process_version_sections (FILE * file)
                ent.vd_aux     = BYTE_GET (edef->vd_aux);
                ent.vd_next    = BYTE_GET (edef->vd_next);
 
-               printf (_("  %#06x: Rev: %d  Flags: %s"),
+               printf (_("  %#06lx: Rev: %d  Flags: %s"),
                        idx, ent.vd_version, get_ver_flags (ent.vd_flags));
 
                printf (_("  Index: %d  Cnt: %d  "),
                        ent.vd_ndx, ent.vd_cnt);
 
-               /* Check for overflow and underflow.  */
-               if (ent.vd_aux + sizeof (* eaux) > (size_t) (endbuf - vstart)
-                   || (vstart + ent.vd_aux < vstart))
+               /* Check for overflow.  */
+               if (vstart + sizeof (*eaux) > endbuf)
+                 break;
+               if (ent.vd_aux > (size_t) (endbuf - (vstart + sizeof (*eaux))))
                  break;
 
                vstart += ent.vd_aux;
@@ -10250,10 +10247,10 @@ process_version_sections (FILE * file)
                    aux.vda_next = BYTE_GET (eaux->vda_next);
 
                    if (VALID_DYNAMIC_NAME (aux.vda_name))
-                     printf (_("  %#06x: Parent %d: %s\n"),
+                     printf (_("  %#06lx: Parent %d: %s\n"),
                              isum, j, GET_DYNAMIC_NAME (aux.vda_name));
                    else
-                     printf (_("  %#06x: Parent %d, name index: %ld\n"),
+                     printf (_("  %#06lx: Parent %d, name index: %ld\n"),
                              isum, j, aux.vda_name);
                  }
 
@@ -10262,7 +10259,7 @@ process_version_sections (FILE * file)
 
                /* PR 17531:
                   file: id:000001,src:000172+005151,op:splice,rep:2.  */
-               if (idx + ent.vd_next < idx)
+               if (ent.vd_next > (size_t) (endbuf - ((char *) edefs + idx)))
                  break;
 
                idx += ent.vd_next;
@@ -10278,8 +10275,8 @@ process_version_sections (FILE * file)
        case SHT_GNU_verneed:
          {
            Elf_External_Verneed * eneed;
-           unsigned int idx;
-           unsigned int cnt;
+           unsigned long idx;
+           unsigned long cnt;
            char * endbuf;
 
            found = TRUE;
@@ -10305,13 +10302,10 @@ process_version_sections (FILE * file)
              {
                Elf_External_Verneed * entry;
                Elf_Internal_Verneed ent;
-               unsigned int isum;
+               unsigned long isum;
                int j;
                char * vstart;
 
-               if (idx > (size_t) (endbuf - (char *) eneed))
-                 break;
-
                vstart = ((char *) eneed) + idx;
                if (vstart + sizeof (*entry) > endbuf)
                  break;
@@ -10324,7 +10318,7 @@ process_version_sections (FILE * file)
                ent.vn_aux     = BYTE_GET (entry->vn_aux);
                ent.vn_next    = BYTE_GET (entry->vn_next);
 
-               printf (_("  %#06x: Version: %d"), idx, ent.vn_version);
+               printf (_("  %#06lx: Version: %d"), idx, ent.vn_version);
 
                if (VALID_DYNAMIC_NAME (ent.vn_file))
                  printf (_("  File: %s"), GET_DYNAMIC_NAME (ent.vn_file));
@@ -10354,10 +10348,10 @@ process_version_sections (FILE * file)
                    aux.vna_next  = BYTE_GET (eaux->vna_next);
 
                    if (VALID_DYNAMIC_NAME (aux.vna_name))
-                     printf (_("  %#06x:   Name: %s"),
+                     printf (_("  %#06lx:   Name: %s"),
                              isum, GET_DYNAMIC_NAME (aux.vna_name));
                    else
-                     printf (_("  %#06x:   Name index: %lx"),
+                     printf (_("  %#06lx:   Name index: %lx"),
                              isum, aux.vna_name);
 
                    printf (_("  Flags: %s  Version: %d\n"),
@@ -10379,9 +10373,10 @@ process_version_sections (FILE * file)
                if (j < ent.vn_cnt)
                  warn (_("Missing Version Needs auxillary information\n"));
 
-               if (ent.vn_next == 0 && cnt < section->sh_info - 1)
+               if (ent.vn_next > (size_t) (endbuf - ((char *) eneed + idx))
+                   || (ent.vn_next == 0 && cnt < section->sh_info - 1))
                  {
-                   warn (_("Corrupt Version Needs structure - offset to next structure is zero with entries still left to be processed\n"));
+                   warn (_("Invalid vn_next field of %lx\n"), ent.vn_next);
                    cnt = section->sh_info;
                    break;
                  }