From: Alan Modra <amodra@gmail.com>
Date: Thu, 6 Oct 2022 23:53:05 +0000 (+1030)
Subject: PR29653, objcopy/strip: fuzzed small input file induces large output file
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ea4e4a19b7f6c192c307b5a37c67d141f3aea074;p=binutils-gdb.git

PR29653, objcopy/strip: fuzzed small input file induces large output file

_bfd_check_format functions should not print errors or warnings if
they return NULL.  A NULL return means the particular target under
test does not match, so there isn't any reason to make a complaint
about the target.  In fact there isn't a good reason to warn even if
the target matches, except via the _bfd_per_xvec_warn mechanism; Some
other target might be a better match.

This patch tidies pe_bfd_object_p with the above in mind, and
restricts the PE optional header SectionAlignment and FileAlignment
fields somewhat.  I chose to warn on nonsense values rather than
refusing to match.  Refusing to match would be OK too.

	PR 29653
	* peXXigen.c (_bfd_XXi_swap_aouthdr_in): Don't emit error about
	invalid NumberOfRvaAndSizes here.  Limit loop copying data
	directory to IMAGE_NUMBEROF_DIRECTORY_ENTRIES.
	* peicode.h (pe_bfd_object_p): Don't clear and test bfd_error
	around bfd_coff_swap_aouthdr_in.  Warn on invalid SectionAlignment,
	FileAlignment and NumberOfRvaAndSizes.  Don't return NULL on
	invalid NumberOfRvaAndSizes.
---

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index a7b85713023..e74ed3968a2 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -517,45 +517,26 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd,
   a->LoaderFlags = H_GET_32 (abfd, src->LoaderFlags);
   a->NumberOfRvaAndSizes = H_GET_32 (abfd, src->NumberOfRvaAndSizes);
 
-  {
-    unsigned idx;
-
-    /* PR 17512: Corrupt PE binaries can cause seg-faults.  */
-    if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
-      {
-	/* xgettext:c-format */
-	_bfd_error_handler
-	  (_("%pB: aout header specifies an invalid number of"
-	     " data-directory entries: %u"), abfd, a->NumberOfRvaAndSizes);
-	bfd_set_error (bfd_error_bad_value);
-
-	/* Paranoia: If the number is corrupt, then assume that the
-	   actual entries themselves might be corrupt as well.  */
-	a->NumberOfRvaAndSizes = 0;
-      }
-
-    for (idx = 0; idx < a->NumberOfRvaAndSizes; idx++)
-      {
-	/* If data directory is empty, rva also should be 0.  */
-	int size =
-	  H_GET_32 (abfd, src->DataDirectory[idx][1]);
-
-	a->DataDirectory[idx].Size = size;
+  /* PR 17512: Don't blindly trust NumberOfRvaAndSizes.  */
+  unsigned idx;
+  for (idx = 0;
+       idx < a->NumberOfRvaAndSizes && idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES;
+       idx++)
+    {
+      /* If data directory is empty, rva also should be 0.  */
+      int size = H_GET_32 (abfd, src->DataDirectory[idx][1]);
+      int vma = size ? H_GET_32 (abfd, src->DataDirectory[idx][0]) : 0;
 
-	if (size)
-	  a->DataDirectory[idx].VirtualAddress =
-	    H_GET_32 (abfd, src->DataDirectory[idx][0]);
-	else
-	  a->DataDirectory[idx].VirtualAddress = 0;
-      }
+      a->DataDirectory[idx].Size = size;
+      a->DataDirectory[idx].VirtualAddress = vma;
+    }
 
-    while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
-      {
-	a->DataDirectory[idx].Size = 0;
-	a->DataDirectory[idx].VirtualAddress = 0;
-	idx ++;
-      }
-  }
+  while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
+    {
+      a->DataDirectory[idx].Size = 0;
+      a->DataDirectory[idx].VirtualAddress = 0;
+      idx++;
+    }
 
   if (aouthdr_int->entry)
     {
diff --git a/bfd/peicode.h b/bfd/peicode.h
index 54a159f0962..3888dd47cc6 100644
--- a/bfd/peicode.h
+++ b/bfd/peicode.h
@@ -1519,19 +1519,41 @@ pe_bfd_object_p (bfd * abfd)
       if (amt > opt_hdr_size)
 	memset (opthdr + opt_hdr_size, 0, amt - opt_hdr_size);
 
-      bfd_set_error (bfd_error_no_error);
-      bfd_coff_swap_aouthdr_in (abfd, opthdr, & internal_a);
-      if (bfd_get_error () != bfd_error_no_error)
-	return NULL;
-    }
+      bfd_coff_swap_aouthdr_in (abfd, opthdr, &internal_a);
+
+      struct internal_extra_pe_aouthdr *a = &internal_a.pe;
+      if ((a->SectionAlignment & -a->SectionAlignment) != a->SectionAlignment
+	  || a->SectionAlignment >= 0x80000000)
+	{
+	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	  *warn = _("%pB: adjusting invalid SectionAlignment");
+	  a->SectionAlignment &= -a->SectionAlignment;
+	  if (a->SectionAlignment >= 0x80000000)
+	    a->SectionAlignment = 0x40000000;
+	}
+
+      if ((a->FileAlignment & -a->FileAlignment) != a->FileAlignment
+	  || a->FileAlignment > a->SectionAlignment)
+	{
+	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	  *warn = _("%pB: adjusting invalid FileAlignment");
+	  a->FileAlignment &= -a->FileAlignment;
+	  if (a->FileAlignment > a->SectionAlignment)
+	    a->FileAlignment = a->SectionAlignment;
+	}
 
+      if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
+	{
+	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	  *warn = _("%pB: invalid NumberOfRvaAndSizes");
+	}
+    }
 
   result = coff_real_object_p (abfd, internal_f.f_nscns, &internal_f,
 			       (opt_hdr_size != 0
 				? &internal_a
 				: (struct internal_aouthdr *) NULL));
 
-
   if (result)
     {
       /* Now the whole header has been processed, see if there is a build-id */