Re: asan: buffer overflow in peXXigen.c

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.

1 files changed, 52 insertions, 44 deletions

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index d11ea01..50e4fac 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;
 	}
     }