asan: buffer overflow in peXXigen.c

asan: buffer overflow in peXXigen.c

[Alan Modra]

	* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Properly
	sanity check DataDirectory[PE_DEBUG_DATA].Size.

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index c71dacd4bf0..d11ea01c554 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -2937,6 +2937,7 @@ bool
 _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
 {
   pe_data_type *ipe, *ope;
+  bfd_size_type size;
 
   /* One day we may try to grok other private data.  */
   if (ibfd->xvec->flavour != bfd_target_coff_flavour
@@ -2971,7 +2972,8 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
   memcpy (ope->dos_message, ipe->dos_message, sizeof (ope->dos_message));
 
   /* The file offsets contained in the debug directory need rewriting.  */
-  if (ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size != 0)
+  size = ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size;
+  if (size != 0)
     {
       bfd_vma addr = ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].VirtualAddress
 	+ ope->pe_opthdr.ImageBase;
@@ -2980,12 +2982,16 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
 	 representing s_size, not virt_size).  Therefore don't look for the
 	 section containing the first byte, but for that covering the last
 	 one.  */
-      bfd_vma last = addr + ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size - 1;
+      bfd_vma last = addr + size - 1;
       asection *section = find_section_by_vma (obfd, last);
       bfd_byte *data;
+      bfd_vma dataoff = addr - section->vma;
 
       /* PR 17512: file: 0f15796a.  */
-      if (section && addr < section->vma)
+      if (section
+	  && (addr < section->vma
+	      || section->size < dataoff
+	      || section->size - dataoff < size))
 	{
 	  /* xgettext:c-format */
 	  _bfd_error_handler
@@ -3000,7 +3006,7 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
 	{
 	  unsigned int i;
 	  struct external_IMAGE_DEBUG_DIRECTORY *dd =
-	    (struct external_IMAGE_DEBUG_DIRECTORY *)(data + (addr - section->vma));
+	    (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff);
 
 	  for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size
 		 / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++)

-- 
Alan Modra
Australia Development Lab, IBM

Re: asan: buffer overflow in peXXigen.c

[Alan Modra]

In the process of fixing a buffer overflow in commit fe69d4fcf0194a,
I managed to introduce a fairly obvious NULL pointer dereference..

	* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Don't
	segfault on not finding section.  Wrap overlong lines.

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index d11ea01c554..50e4face50c 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -2984,64 +2984,72 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
 	 one.  */
       bfd_vma last = addr + size - 1;
       asection *section = find_section_by_vma (obfd, last);
-      bfd_byte *data;
-      bfd_vma dataoff = addr - section->vma;
 
-      /* PR 17512: file: 0f15796a.  */
-      if (section
-	  && (addr < section->vma
-	      || section->size < dataoff
-	      || section->size - dataoff < size))
+      if (section != NULL)
 	{
-	  /* xgettext:c-format */
-	  _bfd_error_handler
-	    (_("%pB: Data Directory (%lx bytes at %" PRIx64 ") "
-	       "extends across section boundary at %" PRIx64),
-	     obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size,
-	     (uint64_t) addr, (uint64_t) section->vma);
-	  return false;
-	}
+	  bfd_byte *data;
+	  bfd_vma dataoff = addr - section->vma;
 
-      if (section && bfd_malloc_and_get_section (obfd, section, &data))
-	{
-	  unsigned int i;
-	  struct external_IMAGE_DEBUG_DIRECTORY *dd =
-	    (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff);
+	  /* PR 17512: file: 0f15796a.  */
+	  if (addr < section->vma
+	      || section->size < dataoff
+	      || section->size - dataoff < size)
+	    {
+	      /* xgettext:c-format */
+	      _bfd_error_handler
+		(_("%pB: Data Directory (%lx bytes at %" PRIx64 ") "
+		   "extends across section boundary at %" PRIx64),
+		 obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size,
+		 (uint64_t) addr, (uint64_t) section->vma);
+	      return false;
+	    }
 
-	  for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size
-		 / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++)
+	  if (bfd_malloc_and_get_section (obfd, section, &data))
 	    {
-	      asection *ddsection;
-	      struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]);
-	      struct internal_IMAGE_DEBUG_DIRECTORY idd;
+	      unsigned int i;
+	      struct external_IMAGE_DEBUG_DIRECTORY *dd =
+		(struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff);
+
+	      for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size
+		     / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++)
+		{
+		  asection *ddsection;
+		  struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]);
+		  struct internal_IMAGE_DEBUG_DIRECTORY idd;
+		  bfd_vma idd_vma;
 
-	      _bfd_XXi_swap_debugdir_in (obfd, edd, &idd);
+		  _bfd_XXi_swap_debugdir_in (obfd, edd, &idd);
 
-	      if (idd.AddressOfRawData == 0)
-		continue; /* RVA 0 means only offset is valid, not handled yet.  */
+		  /* RVA 0 means only offset is valid, not handled yet.  */
+		  if (idd.AddressOfRawData == 0)
+		    continue;
 
-	      ddsection = find_section_by_vma (obfd, idd.AddressOfRawData + ope->pe_opthdr.ImageBase);
-	      if (!ddsection)
-		continue; /* Not in a section! */
+		  idd_vma = idd.AddressOfRawData + ope->pe_opthdr.ImageBase;
+		  ddsection = find_section_by_vma (obfd, idd_vma);
+		  if (!ddsection)
+		    continue; /* Not in a section! */
 
-	      idd.PointerToRawData = ddsection->filepos + (idd.AddressOfRawData
-							   + ope->pe_opthdr.ImageBase) - ddsection->vma;
+		  idd.PointerToRawData
+		    = ddsection->filepos + idd_vma - ddsection->vma;
+		  _bfd_XXi_swap_debugdir_out (obfd, &idd, edd);
+		}
 
-	      _bfd_XXi_swap_debugdir_out (obfd, &idd, edd);
+	      if (!bfd_set_section_contents (obfd, section, data, 0,
+					     section->size))
+		{
+		  _bfd_error_handler (_("failed to update file offsets"
+					" in debug directory"));
+		  free (data);
+		  return false;
+		}
+	      free (data);
 	    }
-
-	  if (!bfd_set_section_contents (obfd, section, data, 0, section->size))
+	  else
 	    {
-	      _bfd_error_handler (_("failed to update file offsets in debug directory"));
-	      free (data);
+	      _bfd_error_handler (_("%pB: failed to read "
+				    "debug data section"), obfd);
 	      return false;
 	    }
-	  free (data);
-	}
-      else if (section)
-	{
-	  _bfd_error_handler (_("%pB: failed to read debug data section"), obfd);
-	  return false;
 	}
     }
 

-- 
Alan Modra
Australia Development Lab, IBM