[binutils-gdb] Fix: nm: SEGV on unknow address at nm.c:718 in print_symname

[Nick Clifton]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6136093c0d009e3118abb08344e156d91144a046

commit 6136093c0d009e3118abb08344e156d91144a046
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Sep 28 12:37:59 2023 +0100

    Fix: nm: SEGV on unknow address at nm.c:718 in print_symname
    
      PR 30886 * elf-bfd.h (struct elf_obj_tdata): Add dt_strsz field.
      * elf.c (_bfd_elf_get_dynamic_symbols): Add a NUL byte at the end of the string table. Initialise the dt_strsz field. (_bfd_elf_slurp_version_tables): Only free the contents if they were malloc'ed. Add checks before setting string pointers in the dt_strtab buffer.

Diff:
---
 bfd/ChangeLog | 11 +++++++++++
 bfd/elf-bfd.h |  3 ++-
 bfd/elf.c     | 37 +++++++++++++++++++++++++++++--------
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 2eee20fae0c..ed9db240eb5 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,14 @@
+2023-09-28  Nick Clifton  <nickc@redhat.com>
+
+	PR 30886
+	* elf-bfd.h (struct elf_obj_tdata): Add dt_strsz field.
+	* elf.c (_bfd_elf_get_dynamic_symbols): Add a NUL byte at the end
+	of the string table.
+	Initialise the dt_strsz field.
+	(_bfd_elf_slurp_version_tables): Only free the contents if they
+	were malloc'ed.
+	Add checks before setting string pointers in the dt_strtab buffer.
+
 2023-09-27  Nick Clifton  <nickc@redhat.com>
 
 	PR 30885
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 52d4d39b705..335081ec629 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2039,7 +2039,8 @@ struct elf_obj_tdata
   size_t dt_symtab_count;
   size_t dt_verdef_count;
   size_t dt_verneed_count;
-  char *dt_strtab;
+  char * dt_strtab;
+  size_t dt_strsz;
   elf_section_list * symtab_shndx_list;
   bfd_vma gp;				/* The gp value */
   unsigned int gp_size;			/* The gp size */
diff --git a/bfd/elf.c b/bfd/elf.c
index fa8881e8ea6..2bd236b405a 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -2102,9 +2102,11 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
     goto error_return;
 
   /* Dynamic string table must be valid until ABFD is closed.  */
-  strbuf = (char *) _bfd_alloc_and_read (abfd, dt_strsz, dt_strsz);
+  strbuf = (char *) _bfd_alloc_and_read (abfd, dt_strsz + 1, dt_strsz);
   if (strbuf == NULL)
     goto error_return;
+  /* Since this is a string table, make sure that it is terminated.  */
+  strbuf[dt_strsz] = 0;
 
   /* Get the real symbol count from DT_HASH or DT_GNU_HASH.  Prefer
      DT_HASH since it is simpler than DT_GNU_HASH.  */
@@ -2356,6 +2358,7 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
 
  empty_gnu_hash:
   elf_tdata (abfd)->dt_strtab = strbuf;
+  elf_tdata (abfd)->dt_strsz = dt_strsz;
   elf_tdata (abfd)->dt_symtab = isymbuf;
   elf_tdata (abfd)->dt_symtab_count = symcount;
   elf_tdata (abfd)->dt_versym = versym;
@@ -9396,6 +9399,7 @@ bool
 _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 {
   bfd_byte *contents = NULL;
+  bool free_contents = false;
   unsigned int freeidx = 0;
   size_t amt;
 
@@ -9438,6 +9442,7 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 	  if (contents == NULL)
 	    goto error_return_verref;
 
+	  free_contents = true;
 	  verneed_size = hdr->sh_size;
 	  verneed_count = hdr->sh_info;
 	}
@@ -9471,8 +9476,13 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 	  iverneed->vn_bfd = abfd;
 
 	  if (elf_use_dt_symtab_p (abfd))
-	    iverneed->vn_filename
-	      = elf_tdata (abfd)->dt_strtab + iverneed->vn_file;
+	    {
+	      if (iverneed->vn_file < elf_tdata (abfd)->dt_strsz)
+		iverneed->vn_filename
+		  = elf_tdata (abfd)->dt_strtab + iverneed->vn_file;
+	      else
+		iverneed->vn_filename = NULL;
+	    }	
 	  else
 	    iverneed->vn_filename
 	      = bfd_elf_string_from_elf_section (abfd, hdr->sh_link,
@@ -9508,8 +9518,13 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 	      _bfd_elf_swap_vernaux_in (abfd, evernaux, ivernaux);
 
 	      if (elf_use_dt_symtab_p (abfd))
-		ivernaux->vna_nodename
-		  = elf_tdata (abfd)->dt_strtab + ivernaux->vna_name;
+		{
+		  if (ivernaux->vna_name < elf_tdata (abfd)->dt_strsz)
+		    ivernaux->vna_nodename
+		      = elf_tdata (abfd)->dt_strtab + ivernaux->vna_name;
+		  else
+		    ivernaux->vna_nodename = NULL;
+		}
 	      else
 		ivernaux->vna_nodename
 		  = bfd_elf_string_from_elf_section (abfd, hdr->sh_link,
@@ -9703,8 +9718,13 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 	      _bfd_elf_swap_verdaux_in (abfd, everdaux, iverdaux);
 
 	      if (elf_use_dt_symtab_p (abfd))
-		iverdaux->vda_nodename
-		  = elf_tdata (abfd)->dt_strtab + iverdaux->vda_name;
+		{
+		  if (iverdaux->vda_name < elf_tdata (abfd)->dt_strsz)
+		    iverdaux->vda_nodename
+		      = elf_tdata (abfd)->dt_strtab + iverdaux->vda_name;
+		  else
+		    iverdaux->vda_nodename = NULL;
+		}
 	      else
 		iverdaux->vda_nodename
 		  = bfd_elf_string_from_elf_section (abfd, hdr->sh_link,
@@ -9799,7 +9819,8 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
   return true;
 
  error_return:
-  free (contents);
+  if (free_contents)
+    free (contents);
   return false;
 }