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>

1 files changed, 44 insertions, 18 deletions

diff --git a/src/unstrip.c b/src/unstrip.c
index fc878325..c097d460 100644
--- 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,