[binutils-gdb] xcoff buffer overflow

[Alan Modra]

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

commit 1f542289e6762f351c56d2de356f91737c7d1d6f
Author: Alan Modra <amodra@gmail.com>
Date:   Sun Apr 6 18:01:33 2025 +0930

    xcoff buffer overflow
    
    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.

Diff:
---
 bfd/xcofflink.c | 73 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 18 deletions(-)

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);