xcoff buffer overflow

Message ID Z_PctVgpEyjojMCw@squeak.grove.modra.org
State New
Headers
Series xcoff buffer overflow |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply

Commit Message

Alan Modra April 7, 2025, 2:09 p.m. UTC
  Much of the xcoff code is not well protected against fuzzed object file
attacks.  This sanity checks some values in ".loader".

	* xcofflink.c (xcoff_get_ldhdr): New function.
	(_bfd_xcoff_get_dynamic_symtab_upper_bound),
	(_bfd_xcoff_canonicalize_dynamic_symtab),
	(_bfd_xcoff_get_dynamic_reloc_upper_bound),
	(_bfd_xcoff_canonicalize_dynamic_reloc),
	(xcoff_link_add_dynamic_symbols),
	(xcoff_link_check_dynamic_ar_symbols): Use xcoff_get_ldhdr.
  

Patch

diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c
index 446fa5a85f4..5f0efb45df2 100644
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -243,6 +243,55 @@  xcoff_get_section_contents (bfd *abfd, asection *sec)
   return contents;
 }
 
+/* Read .loader and swap in the header.  Sanity check to prevent
+   buffer overflows.  Don't bother to check for overlap as that sort
+   of insanity shouldn't lead to incorrect program behaviour.  */
+
+static bfd_byte *
+xcoff_get_ldhdr (bfd *abfd, asection *lsec, struct internal_ldhdr *ldhdr)
+{
+  bfd_byte *contents = xcoff_get_section_contents (abfd, lsec);
+  if (contents)
+    {
+      bfd_xcoff_swap_ldhdr_in (abfd, contents, ldhdr);
+      if (ldhdr->l_nsyms != 0)
+	{
+	  bfd_vma symoff = bfd_xcoff_loader_symbol_offset (abfd, ldhdr);
+	  if (symoff > lsec->size)
+	    goto fail;
+	  bfd_size_type onesym = bfd_xcoff_ldsymsz (abfd);
+	  bfd_size_type syms;
+	  if (_bfd_mul_overflow (ldhdr->l_nsyms, onesym, &syms)
+	      || syms > lsec->size - symoff)
+	    goto fail;
+	}
+      if (ldhdr->l_stlen != 0
+	  && (ldhdr->l_stoff > lsec->size
+	      || ldhdr->l_stlen > lsec->size - ldhdr->l_stoff))
+	goto fail;
+      if (ldhdr->l_nreloc != 0)
+	{
+	  bfd_vma reloff = bfd_xcoff_loader_reloc_offset (abfd, ldhdr);
+	  if (reloff > lsec->size)
+	    goto fail;
+	  bfd_size_type onerel = bfd_xcoff_ldrelsz (abfd);
+	  bfd_size_type rels;
+	  if (_bfd_mul_overflow (ldhdr->l_nreloc, onerel, &rels)
+	      || rels > lsec->size - reloff)
+	    goto fail;
+	}
+      if (ldhdr->l_nimpid != 0
+	  && (ldhdr->l_impoff > lsec->size
+	      || ldhdr->l_istlen > lsec->size - ldhdr->l_impoff))
+	goto fail;
+    }
+  return contents;
+
+ fail:
+  bfd_set_error (bfd_error_file_truncated);
+  return NULL;
+}
+
 /* Get the size required to hold the dynamic symbols.  */
 
 long
@@ -265,12 +314,10 @@  _bfd_xcoff_get_dynamic_symtab_upper_bound (bfd *abfd)
       return -1;
     }
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return -1;
 
-  bfd_xcoff_swap_ldhdr_in (abfd, (void *) contents, &ldhdr);
-
   return (ldhdr.l_nsyms + 1) * sizeof (asymbol *);
 }
 
@@ -299,12 +346,10 @@  _bfd_xcoff_canonicalize_dynamic_symtab (bfd *abfd, asymbol **psyms)
       return -1;
     }
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return -1;
 
-  bfd_xcoff_swap_ldhdr_in (abfd, contents, &ldhdr);
-
   strings = (char *) contents + ldhdr.l_stoff;
 
   symbuf = bfd_zalloc (abfd, ldhdr.l_nsyms * sizeof (* symbuf));
@@ -384,12 +429,10 @@  _bfd_xcoff_get_dynamic_reloc_upper_bound (bfd *abfd)
       return -1;
     }
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return -1;
 
-  bfd_xcoff_swap_ldhdr_in (abfd, (struct external_ldhdr *) contents, &ldhdr);
-
   return (ldhdr.l_nreloc + 1) * sizeof (arelent *);
 }
 
@@ -419,12 +462,10 @@  _bfd_xcoff_canonicalize_dynamic_reloc (bfd *abfd,
       return -1;
     }
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return -1;
 
-  bfd_xcoff_swap_ldhdr_in (abfd, contents, &ldhdr);
-
   relbuf = bfd_alloc (abfd, ldhdr.l_nreloc * sizeof (arelent));
   if (relbuf == NULL)
     return -1;
@@ -905,7 +946,7 @@  xcoff_link_add_dynamic_symbols (bfd *abfd, struct bfd_link_info *info)
       return false;
     }
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return false;
 
@@ -913,8 +954,6 @@  xcoff_link_add_dynamic_symbols (bfd *abfd, struct bfd_link_info *info)
      included in the link.  */
   bfd_section_list_clear (abfd);
 
-  bfd_xcoff_swap_ldhdr_in (abfd, contents, &ldhdr);
-
   strings = (char *) contents + ldhdr.l_stoff;
 
   elsym = contents + bfd_xcoff_loader_symbol_offset(abfd, &ldhdr);
@@ -2368,12 +2407,10 @@  xcoff_link_check_dynamic_ar_symbols (bfd *abfd,
     /* There are no symbols, so don't try to include it.  */
     return true;
 
-  contents = xcoff_get_section_contents (abfd, lsec);
+  contents = xcoff_get_ldhdr (abfd, lsec, &ldhdr);
   if (!contents)
     return false;
 
-  bfd_xcoff_swap_ldhdr_in (abfd, contents, &ldhdr);
-
   strings = (char *) contents + ldhdr.l_stoff;
 
   elsym = contents + bfd_xcoff_loader_symbol_offset (abfd, &ldhdr);