[binutils-gdb] PR 34053 buffer overflow in xcoff_link_add_symbols

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

commit 23acf2f003f81b2f8d9d1997ea45d822d33d386c
Author: Alan Modra <[email protected]>
Date:   Thu Apr 9 09:06:27 2026 +0930

    PR 34053 buffer overflow in xcoff_link_add_symbols
    
    This patch adds two sanity checks with error reporting in
    xcoff_link_add_symbols before reading symbol aux entries, add extends
    assertions in later functions.  A whole lot of unnecessary casts are
    also tidied.
    
            PR 34053
            * xcofflink.c: Remove unnecessary casts throughout.
            (xcoff_link_add_symbols): Sanity check aux entries are within
            symbol buffer.
            (bfd_xcoff_build_dynamic_sections): Assert the above is true.
            (xcoff_link_input_bfd): Likewise.

Diff:
---
 bfd/xcofflink.c | 132 ++++++++++++++++++++++++++------------------------------
 1 file changed, 62 insertions(+), 70 deletions(-)

diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c
index 7f1c0df760f..cf3b33e7202 100644
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -371,7 +371,7 @@ _bfd_xcoff_canonicalize_dynamic_symtab (bfd *abfd, asymbol **psyms)
 	{
 	  char *c;
 
-	  c = bfd_alloc (abfd, (bfd_size_type) SYMNMLEN + 1);
+	  c = bfd_alloc (abfd, SYMNMLEN + 1);
 	  if (c == NULL)
 	    return -1;
 	  memcpy (c, ldsym._l._l_name, SYMNMLEN);
@@ -1038,7 +1038,7 @@ xcoff_link_add_dynamic_symbols (bfd *abfd, struct bfd_link_info *info)
 	    {
 	      char *dsnm;
 
-	      dsnm = bfd_malloc ((bfd_size_type) strlen (name) + 2);
+	      dsnm = bfd_malloc (strlen (name) + 2);
 	      if (dsnm == NULL)
 		return false;
 	      dsnm[0] = '.';
@@ -1081,7 +1081,7 @@ xcoff_link_add_dynamic_symbols (bfd *abfd, struct bfd_link_info *info)
   coff_section_data (abfd, lsec)->contents = NULL;
 
   /* Record this file in the import files.  */
-  n = bfd_alloc (abfd, (bfd_size_type) sizeof (struct xcoff_import_file));
+  n = bfd_alloc (abfd, sizeof (*n));
   if (n == NULL)
     return false;
   n->next = NULL;
@@ -1477,7 +1477,7 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
       bfd_vma value;
       struct xcoff_link_hash_entry *set_toc;
 
-      bfd_coff_swap_sym_in (abfd, (void *) esym, (void *) &sym);
+      bfd_coff_swap_sym_in (abfd, esym, &sym);
 
       /* In this pass we are only interested in symbols with csect
 	 information.  */
@@ -1523,9 +1523,12 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
 	{
 	  union internal_auxent auxlin;
 
-	  bfd_coff_swap_aux_in (abfd, (void *) (esym + symesz),
+	  if (symesz >= (size_t) (esym_end - esym))
+	    goto badaux;
+
+	  bfd_coff_swap_aux_in (abfd, esym + symesz,
 				sym.n_type, sym.n_sclass,
-				0, sym.n_numaux, (void *) &auxlin);
+				0, sym.n_numaux, &auxlin);
 
 	  if (auxlin.x_sym.x_fcnary.x_fcn.x_lnnoptr != 0)
 	    {
@@ -1552,7 +1555,7 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
 
 		  linpstart = (reloc_info[enclosing->target_index].linenos
 			       + linoff);
-		  bfd_coff_swap_lineno_in (abfd, (void *) linpstart, (void *) &lin);
+		  bfd_coff_swap_lineno_in (abfd, linpstart, &lin);
 		  if (lin.l_lnno == 0
 		      && ((bfd_size_type) lin.l_addr.l_symndx
 			  == ((esym
@@ -1567,8 +1570,7 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
 			   linp < linpend;
 			   linp += linesz)
 			{
-			  bfd_coff_swap_lineno_in (abfd, (void *) linp,
-						   (void *) &lin);
+			  bfd_coff_swap_lineno_in (abfd, linp, &lin);
 			  if (lin.l_lnno == 0)
 			    break;
 			}
@@ -1589,21 +1591,21 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
       visibility = sym.n_type & SYM_V_MASK;
 
       /* Pick up the csect auxiliary information.  */
-      if (sym.n_numaux == 0)
+      if (sym.n_numaux < 1
+	  || sym.n_numaux * symesz >= (size_t) (esym_end - esym))
 	{
+	badaux:
 	  _bfd_error_handler
 	    /* xgettext:c-format */
-	    (_("%pB: class %d symbol `%s' has no aux entries"),
+	    (_("%pB: class %d symbol '%s' has missing aux entries"),
 	     abfd, sym.n_sclass, name);
 	  bfd_set_error (bfd_error_bad_value);
 	  goto error_return;
 	}
 
-      bfd_coff_swap_aux_in (abfd,
-			    (void *) (esym + symesz * sym.n_numaux),
+      bfd_coff_swap_aux_in (abfd, esym + symesz * sym.n_numaux,
 			    sym.n_type, sym.n_sclass,
-			    sym.n_numaux - 1, sym.n_numaux,
-			    (void *) &aux);
+			    sym.n_numaux - 1, sym.n_numaux, &aux);
 
       smtyp = SMTYP_SMTYP (aux.x_csect.x_smtyp);
 
@@ -1726,7 +1728,7 @@ xcoff_link_add_symbols (bfd *abfd, struct bfd_link_info *info)
 
 		  erelsym = ((bfd_byte *) obj_coff_external_syms (abfd)
 			     + rel->r_symndx * symesz);
-		  bfd_coff_swap_sym_in (abfd, (void *) erelsym, (void *) &relsym);
+		  bfd_coff_swap_sym_in (abfd, erelsym, &relsym);
 		  if (EXTERN_SYM_P (relsym.n_sclass))
 		    {
 		      const char *relname;
@@ -2507,7 +2509,7 @@ xcoff_link_check_ar_symbols (bfd *abfd,
     {
       struct internal_syment sym;
 
-      bfd_coff_swap_sym_in (abfd, (void *) esym, (void *) &sym);
+      bfd_coff_swap_sym_in (abfd, esym, &sym);
       esym += (sym.n_numaux + 1) * symesz;
 
       if (EXTERN_SYM_P (sym.n_sclass) && sym.n_scnum != N_UNDEF)
@@ -4005,7 +4007,7 @@ bfd_xcoff_size_dynamic_sections (bfd *output_bfd,
     return true;
 
   xcoff_link_hash_traverse (xcoff_hash_table (info), xcoff_post_gc_symbol,
-			    (void *) ldinfo);
+			    ldinfo);
   if (ldinfo->failed)
     goto error_return;
 
@@ -4216,7 +4218,8 @@ bfd_xcoff_build_dynamic_sections (bfd *output_bfd,
 	  /* Read in the csect information, if any.  */
 	  if (CSECT_SYM_P (sym.n_sclass))
 	    {
-	      BFD_ASSERT (sym.n_numaux > 0);
+	      BFD_ASSERT (sym.n_numaux > 0
+			  && symesz * sym.n_numaux < (size_t) (esymend - esym));
 	      bfd_coff_swap_aux_in (sub, esym + symesz * sym.n_numaux,
 				    sym.n_type, sym.n_sclass,
 				    sym.n_numaux - 1, sym.n_numaux, &aux);
@@ -4307,7 +4310,7 @@ bfd_xcoff_link_generate_rtinit (bfd *abfd,
 {
   struct bfd_in_memory *bim;
 
-  bim = bfd_malloc ((bfd_size_type) sizeof (* bim));
+  bim = bfd_malloc (sizeof (*bim));
   if (bim == NULL)
     return false;
 
@@ -4316,7 +4319,7 @@ bfd_xcoff_link_generate_rtinit (bfd *abfd,
 
   abfd->link.next = 0;
   abfd->format = bfd_object;
-  abfd->iostream = (void *) bim;
+  abfd->iostream = bim;
   abfd->flags = BFD_IN_MEMORY;
   abfd->iovec = &_bfd_memory_iovec;
   abfd->direction = write_direction;
@@ -4876,8 +4879,8 @@ bfd_xcoff_size_stubs (struct bfd_link_info *info)
 			}
 
 		      bfd_coff_swap_sym_in (input_bfd,
-					    (void *) esyms + irel->r_symndx * symesz,
-					    (void *) &sym);
+					    esyms + irel->r_symndx * symesz,
+					    &sym);
 
 		      sym_sec = xcoff_data (input_bfd)->csects[irel->r_symndx];
 		      sym_value = sym.n_value - sym_sec->vma;
@@ -5250,17 +5253,16 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
       int smtyp = 0;
       int add;
 
-      bfd_coff_swap_sym_in (input_bfd, (void *) esym, (void *) isymp);
+      bfd_coff_swap_sym_in (input_bfd, esym, isymp);
 
       /* Read in the csect information, if any.  */
       if (CSECT_SYM_P (isymp->n_sclass))
 	{
-	  BFD_ASSERT (isymp->n_numaux > 0);
-	  bfd_coff_swap_aux_in (input_bfd,
-				(void *) (esym + isymesz * isymp->n_numaux),
+	  BFD_ASSERT (isymp->n_numaux > 0
+		      && isymesz * isymp->n_numaux < (size_t) (esym_end - esym));
+	  bfd_coff_swap_aux_in (input_bfd, esym + isymesz * isymp->n_numaux,
 				isymp->n_type, isymp->n_sclass,
-				isymp->n_numaux - 1, isymp->n_numaux,
-				(void *) &aux);
+				isymp->n_numaux - 1, isymp->n_numaux, &aux);
 
 	  smtyp = SMTYP_SMTYP (aux.x_csect.x_smtyp);
 	}
@@ -5475,12 +5477,10 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 		  if ((bfd_size_type) flinfo->last_file_index >= syment_base)
 		    {
 		      /* The last C_FILE symbol is in this input file.  */
-		      bfd_coff_swap_sym_out (output_bfd,
-					     (void *) &flinfo->last_file,
-					     (void *) (flinfo->outsyms
-						    + ((flinfo->last_file_index
-							- syment_base)
-						       * osymesz)));
+		      bfd_coff_swap_sym_out
+			(output_bfd, &flinfo->last_file,
+			 flinfo->outsyms + (flinfo->last_file_index
+					    - syment_base) * osymesz);
 		    }
 		  else
 		    {
@@ -5489,9 +5489,8 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 			 borrow *outsym temporarily.  */
 		      file_ptr pos;
 
-		      bfd_coff_swap_sym_out (output_bfd,
-					     (void *) &flinfo->last_file,
-					     (void *) outsym);
+		      bfd_coff_swap_sym_out (output_bfd, &flinfo->last_file,
+					     outsym);
 
 		      pos = obj_sym_filepos (output_bfd);
 		      pos += flinfo->last_file_index * osymesz;
@@ -5557,7 +5556,7 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 	    }
 
 	  /* Output the symbol.  */
-	  bfd_coff_swap_sym_out (output_bfd, (void *) &isym, (void *) outsym);
+	  bfd_coff_swap_sym_out (output_bfd, &isym, outsym);
 
 	  esym += isymesz;
 	  outsym += osymesz;
@@ -5566,9 +5565,9 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 	    {
 	      union internal_auxent aux;
 
-	      bfd_coff_swap_aux_in (input_bfd, (void *) esym, isymp->n_type,
-				    isymp->n_sclass, i, isymp->n_numaux,
-				    (void *) &aux);
+	      bfd_coff_swap_aux_in (input_bfd, esym,
+				    isymp->n_type, isymp->n_sclass, i,
+				    isymp->n_numaux, &aux);
 
 	      if (isymp->n_sclass == C_FILE)
 		{
@@ -5796,9 +5795,9 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 		    }
 		}
 
-	      bfd_coff_swap_aux_out (output_bfd, (void *) &aux, isymp->n_type,
+	      bfd_coff_swap_aux_out (output_bfd, &aux, isymp->n_type,
 				     isymp->n_sclass, i, isymp->n_numaux,
-				     (void *) outsym);
+				     outsym);
 	      outsym += osymesz;
 	      esym += isymesz;
 	    }
@@ -5820,10 +5819,9 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
       && (bfd_size_type) flinfo->last_file_index >= syment_base)
     {
       flinfo->last_file.n_value = output_index;
-      bfd_coff_swap_sym_out (output_bfd, (void *) &flinfo->last_file,
-			     (void *) (flinfo->outsyms
-				    + ((flinfo->last_file_index - syment_base)
-				       * osymesz)));
+      bfd_coff_swap_sym_out (output_bfd, &flinfo->last_file,
+			     flinfo->outsyms + (flinfo->last_file_index
+						- syment_base) * osymesz);
     }
 
   /* Write the modified symbols to the output file.  */
@@ -6036,16 +6034,13 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo,
 			      void * auxptr;
 			      union internal_auxent aux;
 
-			      auxptr = ((void *)
-					(((bfd_byte *)
-					  obj_coff_external_syms (input_bfd))
-					 + ((r_symndx + is->n_numaux)
-					    * isymesz)));
+			      auxptr = ((bfd_byte *)
+					obj_coff_external_syms (input_bfd)
+					+ (r_symndx + is->n_numaux) * isymesz);
 			      bfd_coff_swap_aux_in (input_bfd, auxptr,
 						    is->n_type, is->n_sclass,
 						    is->n_numaux - 1,
-						    is->n_numaux,
-						    (void *) &aux);
+						    is->n_numaux, &aux);
 			      if (SMTYP_SMTYP (aux.x_csect.x_smtyp) == XTY_SD
 				  && aux.x_csect.x_smclas == XMC_TC0)
 				indx = flinfo->toc_symindx;
@@ -6564,12 +6559,12 @@ xcoff_write_global_symbol (struct bfd_hash_entry *bh, void * inf)
 	  irsym.n_type = T_NULL;
 	  irsym.n_numaux = 1;
 
-	  bfd_coff_swap_sym_out (output_bfd, (void *) &irsym, (void *) outsym);
+	  bfd_coff_swap_sym_out (output_bfd, &irsym, outsym);
 	  outsym += bfd_coff_symesz (output_bfd);
 
 	  /* Note : iraux is initialized above.  */
-	  bfd_coff_swap_aux_out (output_bfd, (void *) &iraux, T_NULL, C_HIDEXT,
-				 0, 1, (void *) outsym);
+	  bfd_coff_swap_aux_out (output_bfd, &iraux, T_NULL, C_HIDEXT,
+				 0, 1, outsym);
 	  outsym += bfd_coff_auxesz (output_bfd);
 
 	  if (h->indx >= 0)
@@ -6807,12 +6802,11 @@ xcoff_write_global_symbol (struct bfd_hash_entry *bh, void * inf)
   isym.n_type = T_NULL;
   isym.n_numaux = 1;
 
-  bfd_coff_swap_sym_out (output_bfd, (void *) &isym, (void *) outsym);
+  bfd_coff_swap_sym_out (output_bfd, &isym, outsym);
   outsym += bfd_coff_symesz (output_bfd);
 
   aux.x_csect.x_smclas = h->smclas;
-  bfd_coff_swap_aux_out (output_bfd, (void *) &aux, T_NULL, isym.n_sclass, 0, 1,
-			 (void *) outsym);
+  bfd_coff_swap_aux_out (output_bfd, &aux, T_NULL, isym.n_sclass, 0, 1, outsym);
   outsym += bfd_coff_auxesz (output_bfd);
 
   if ((h->root.type == bfd_link_hash_defined
@@ -6827,13 +6821,12 @@ xcoff_write_global_symbol (struct bfd_hash_entry *bh, void * inf)
 	isym.n_sclass = C_WEAKEXT;
       else
 	isym.n_sclass = C_EXT;
-      bfd_coff_swap_sym_out (output_bfd, (void *) &isym, (void *) outsym);
+      bfd_coff_swap_sym_out (output_bfd, &isym, outsym);
       outsym += bfd_coff_symesz (output_bfd);
 
       aux.x_csect.x_smtyp = XTY_LD;
       aux.x_csect.x_scnlen.u64 = obj_raw_syment_count (output_bfd);
-      bfd_coff_swap_aux_out (output_bfd, (void *) &aux, T_NULL, C_EXT, 0, 1,
-			     (void *) outsym);
+      bfd_coff_swap_aux_out (output_bfd, &aux, T_NULL, C_EXT, 0, 1, outsym);
       outsym += bfd_coff_auxesz (output_bfd);
     }
 
@@ -6929,8 +6922,8 @@ xcoff_reloc_link_order (bfd *output_bfd,
 	     howto->name, addend, NULL, NULL, (bfd_vma) 0);
 	  break;
 	}
-      ok = bfd_set_section_contents (output_bfd, output_section, (void *) buf,
-				     (file_ptr) link_order->offset, size);
+      ok = bfd_set_section_contents (output_bfd, output_section, buf,
+				     link_order->offset, size);
       free (buf);
       if (! ok)
 	return false;
@@ -7395,8 +7388,7 @@ _bfd_xcoff_bfd_final_link (bfd *abfd, struct bfd_link_info *info)
   if (flinfo.last_file_index != -1)
     {
       flinfo.last_file.n_value = -(bfd_vma) 1;
-      bfd_coff_swap_sym_out (abfd, (void *) &flinfo.last_file,
-			     (void *) flinfo.outsyms);
+      bfd_coff_swap_sym_out (abfd, &flinfo.last_file, flinfo.outsyms);
       pos = obj_sym_filepos (abfd) + flinfo.last_file_index * symesz;
       if (bfd_seek (abfd, pos, SEEK_SET) != 0
 	  || bfd_write (flinfo.outsyms, symesz, abfd) != symesz)
@@ -7480,7 +7472,7 @@ _bfd_xcoff_bfd_final_link (bfd *abfd, struct bfd_link_info *info)
 	 appear in the symbol table, which is not necessarily by
 	 address.  So we sort them here.  There may be a better way to
 	 do this.  */
-      qsort ((void *) flinfo.section_info[o->target_index].relocs,
+      qsort (flinfo.section_info[o->target_index].relocs,
 	     o->reloc_count, sizeof (struct internal_reloc),
 	     xcoff_sort_relocs);
 
@@ -7488,7 +7480,7 @@ _bfd_xcoff_bfd_final_link (bfd *abfd, struct bfd_link_info *info)
       irelend = irel + o->reloc_count;
       erel = external_relocs;
       for (; irel < irelend; irel++, rel_hash++, erel += relsz)
-	bfd_coff_swap_reloc_out (abfd, (void *) irel, (void *) erel);
+	bfd_coff_swap_reloc_out (abfd, irel, erel);
 
       rel_size = relsz * o->reloc_count;
       if (bfd_seek (abfd, o->rel_filepos, SEEK_SET) != 0