unstrip: Add various checks for bad input data.

There were various ways to crash eu-unstrip with bad ELF input
data. Add various tests against bad data and allocate some structures
on the heap instead of on the stack.

https://sourceware.org/bugzilla/show_bug.cgi?id=25082

Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/src/ChangeLog b/src/ChangeLog
index db56a136c0b7fe09cc5c5c500c5a24b9216c4d5b..9a7d9558ddaf130f384c3923ace3c0e254ced1cf 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,21 @@
+2019-10-21  Mark Wielaard  <mark@klomp.org>
+
+       * unstrip.c (adjust_relocs): Add map_size argument and check ndx
+       against it.
+       (adjust_all_relocs): Add map_size argument and pass it to
+       adjust_relocs.
+       (add_new_section_symbols): Call adjust_all_relocs with symndx_map
+       size.
+       (collect)symbols): Check sym and string data can be found.
+       (compare_symbols_output): Call error when (different) symbols are
+       equal.
+       (new_shstrtab): Make unstripped_strent array one larger. Check
+       stripped_shnum isn't zero.
+       (copy_elided_sections): Add ndx_sec_num as size of ndx_section
+       array. Check sh_link and sh_info are not larger than ndx_sec_num.
+       Allocate symbols and symndx_map arrays on heap, not stack. Pass
+       map sizes to adjust_all_relocs.
+
 2019-09-28  Dmitry V. Levin  <ldv@altlinux.org>
 
        * elflint.c (main): When an input file cannot be opened,

diff --git a/src/unstrip.c b/src/unstrip.c
index fc87832550b38f250d6a71a4967afe8519d222c7..c097d46026d28a3889729d24bad10423f479eae4 100644 (file)
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -433,7 +433,7 @@ update_sh_size (Elf_Scn *outscn, const Elf_Data *data)
 /* Update relocation sections using the symbol table.  */
 static void
 adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
-              size_t map[], const GElf_Shdr *symshdr)
+              size_t map[], size_t map_size, const GElf_Shdr *symshdr)
 {
   Elf_Data *data = elf_getdata (outscn, NULL);
 
@@ -441,7 +441,11 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
     {
       size_t ndx = GELF_R_SYM (*info);
       if (ndx != STN_UNDEF)
-       *info = GELF_R_INFO (map[ndx - 1], GELF_R_TYPE (*info));
+       {
+         if (ndx > map_size)
+           error (EXIT_FAILURE, 0, "bad symbol ndx section");
+         *info = GELF_R_INFO (map[ndx - 1], GELF_R_TYPE (*info));
+       }
     }
 
   switch (shdr->sh_type)
@@ -588,7 +592,7 @@ adjust_relocs (Elf_Scn *outscn, Elf_Scn *inscn, const GElf_Shdr *shdr,
 /* Adjust all the relocation sections in the file.  */
 static void
 adjust_all_relocs (Elf *elf, Elf_Scn *symtab, const GElf_Shdr *symshdr,
-                  size_t map[])
+                  size_t map[], size_t map_size)
 {
   size_t new_sh_link = elf_ndxscn (symtab);
   Elf_Scn *scn = NULL;
@@ -603,7 +607,7 @@ adjust_all_relocs (Elf *elf, Elf_Scn *symtab, const GElf_Shdr *symshdr,
           stripped_symtab.  */
        if (shdr->sh_type != SHT_NOBITS && shdr->sh_type != SHT_GROUP
            && shdr->sh_link == new_sh_link)
-         adjust_relocs (scn, scn, shdr, map, symshdr);
+         adjust_relocs (scn, scn, shdr, map, map_size, symshdr);
       }
 }
 
@@ -687,7 +691,7 @@ add_new_section_symbols (Elf_Scn *old_symscn, size_t old_shnum,
     }
 
   /* Adjust any relocations referring to the old symbol table.  */
-  adjust_all_relocs (elf, symscn, shdr, symndx_map);
+  adjust_all_relocs (elf, symscn, shdr, symndx_map, nsym - 1);
 
   return symdata;
 }
@@ -835,7 +839,9 @@ collect_symbols (Elf *outelf, bool rel, Elf_Scn *symscn, Elf_Scn *strscn,
                 struct section *split_bss)
 {
   Elf_Data *symdata = elf_getdata (symscn, NULL);
+  ELF_CHECK (symdata != NULL, _("cannot get symbol section data: %s"));
   Elf_Data *strdata = elf_getdata (strscn, NULL);
+  ELF_CHECK (strdata != NULL, _("cannot get string section data: %s"));
   Elf_Data *shndxdata = NULL;  /* XXX */
 
   for (size_t i = 1; i < nent; ++i)
@@ -931,14 +937,14 @@ compare_symbols_output (const void *a, const void *b)
        {
          /* binutils always puts section symbols in section index order.  */
          CMP (shndx);
-         else
-           assert (s1 == s2);
+         else if (s1 != s2)
+           error (EXIT_FAILURE, 0, "section symbols in unexpected order");
        }
 
       /* Nothing really matters, so preserve the original order.  */
       CMP (map);
-      else
-       assert (s1 == s2);
+      else if (s1 != s2)
+       error (EXIT_FAILURE, 0, "found two identical symbols");
     }
 
   return cmp;
@@ -1305,7 +1311,7 @@ new_shstrtab (Elf *unstripped, size_t unstripped_shnum,
   if (strtab == NULL)
     return NULL;
 
-  Dwelf_Strent *unstripped_strent[unstripped_shnum - 1];
+  Dwelf_Strent *unstripped_strent[unstripped_shnum];
   memset (unstripped_strent, 0, sizeof unstripped_strent);
   for (struct section *sec = sections;
        sec < &sections[stripped_shnum - 1];
@@ -1388,6 +1394,9 @@ copy_elided_sections (Elf *unstripped, Elf *stripped,
     error (EXIT_FAILURE, 0, _("\
 more sections in stripped file than debug file -- arguments reversed?"));
 
+  if (unlikely (stripped_shnum == 0))
+    error (EXIT_FAILURE, 0, _("no sections in stripped file"));
+
   /* Cache the stripped file's section details.  */
   struct section sections[stripped_shnum - 1];
   Elf_Scn *scn = NULL;
@@ -1550,10 +1559,11 @@ more sections in stripped file than debug file -- arguments reversed?"));
   /* Make sure each main file section has a place to go.  */
   const struct section *stripped_dynsym = NULL;
   size_t debuglink = SHN_UNDEF;
-  size_t ndx_section[stripped_shnum - 1];
+  size_t ndx_sec_num = stripped_shnum - 1;
+  size_t ndx_section[ndx_sec_num];
   Dwelf_Strtab *strtab = NULL;
   for (struct section *sec = sections;
-       sec < &sections[stripped_shnum - 1];
+       sec < &sections[ndx_sec_num];
        ++sec)
     {
       size_t secndx = elf_ndxscn (sec->scn);
@@ -1658,9 +1668,21 @@ more sections in stripped file than debug file -- arguments reversed?"));
          shdr_mem.sh_flags |= SHF_INFO_LINK;
 
        if (sec->shdr.sh_link != SHN_UNDEF)
-         shdr_mem.sh_link = ndx_section[sec->shdr.sh_link - 1];
+         {
+           if (sec->shdr.sh_link > ndx_sec_num)
+             error (EXIT_FAILURE, 0,
+                    "section [%zd] has invalid sh_link %" PRId32,
+                    elf_ndxscn (sec->scn), sec->shdr.sh_link);
+           shdr_mem.sh_link = ndx_section[sec->shdr.sh_link - 1];
+         }
        if (SH_INFO_LINK_P (&sec->shdr) && sec->shdr.sh_info != 0)
-         shdr_mem.sh_info = ndx_section[sec->shdr.sh_info - 1];
+         {
+           if (sec->shdr.sh_info > ndx_sec_num)
+             error (EXIT_FAILURE, 0,
+                    "section [%zd] has invalid sh_info %" PRId32,
+                    elf_ndxscn (sec->scn), sec->shdr.sh_info);
+           shdr_mem.sh_info = ndx_section[sec->shdr.sh_info - 1];
+         }
 
        if (strtab != NULL)
          shdr_mem.sh_name = dwelf_strent_off (sec->strent);
@@ -1776,8 +1798,8 @@ more sections in stripped file than debug file -- arguments reversed?"));
       /* First collect all the symbols from both tables.  */
 
       const size_t total_syms = stripped_nsym - 1 + unstripped_nsym - 1;
-      struct symbol symbols[total_syms];
-      size_t symndx_map[total_syms];
+      struct symbol *symbols = xmalloc (total_syms * sizeof (struct symbol));
+      size_t *symndx_map = xmalloc (total_syms * sizeof (size_t));
 
       if (stripped_symtab != NULL)
        collect_symbols (unstripped, stripped_ehdr->e_type == ET_REL,
@@ -1958,12 +1980,16 @@ more sections in stripped file than debug file -- arguments reversed?"));
               ++sec)
            if (sec->outscn != NULL && sec->shdr.sh_link == old_sh_link)
              adjust_relocs (sec->outscn, sec->scn, &sec->shdr,
-                            symndx_map, shdr);
+                            symndx_map, total_syms, shdr);
        }
 
       /* Also adjust references to the other old symbol table.  */
       adjust_all_relocs (unstripped, unstripped_symtab, shdr,
-                        &symndx_map[stripped_nsym - 1]);
+                        &symndx_map[stripped_nsym - 1],
+                        total_syms - (stripped_nsym - 1));
+
+      free (symbols);
+      free (symndx_map);
     }
   else if (stripped_symtab != NULL && stripped_shnum != unstripped_shnum)
     check_symtab_section_symbols (unstripped,