peXXigen.c sanity checks

peXXigen.c sanity checks

[Alan Modra]

Also fix a memory leak, and make some style changes.  I tend to read
(sizeof * x) as a multiplication of two variables, which I would not
do if binutils followed the gcc coding conventions consistently (see
https://gcc.gnu.org/codingconventions.html#Expressions).  (sizeof *x)
looks a lot better to me, or even (sizeof (*x)) which I've used here.

	* peXXigen.c (get_contents_sanity_check): New function.
	(pe_print_idata): Use it here..
	(pe_print_edata): ..and here.  Free data on error return.
	(rsrc_parse_entry): Check entry size read from file.
	(rsrc_parse_entries): Style fixes.
	(rsrc_process_section): Use bfd_malloc_and_get_section.
	(_bfd_XXi_final_link_postscript): Likewise.

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 292f7f76a0f..fa2b4296e86 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -1247,6 +1247,24 @@ static char * dir_names[IMAGE_NUMBEROF_DIRECTORY_ENTRIES] =
   N_("Reserved")
 };
 
+static bool
+get_contents_sanity_check (bfd *abfd, asection *section,
+			   bfd_size_type dataoff, bfd_size_type datasize)
+{
+  if ((section->flags & SEC_HAS_CONTENTS) == 0)
+    return false;
+  if (dataoff > section->size
+      || datasize > section->size - dataoff)
+    return false;
+  ufile_ptr filesize = bfd_get_file_size (abfd);
+  if (filesize != 0
+      && ((ufile_ptr) section->filepos > filesize
+	  || dataoff > filesize - section->filepos
+	  || datasize > filesize - section->filepos - dataoff))
+    return false;
+  return true;
+}
+
 static bool
 pe_print_idata (bfd * abfd, void * vfile)
 {
@@ -1413,6 +1431,9 @@ pe_print_idata (bfd * abfd, void * vfile)
 		{
 		  ft_idx = first_thunk - (ft_section->vma - extra->ImageBase);
 		  ft_datasize = ft_section->size - ft_idx;
+		  if (!get_contents_sanity_check (abfd, ft_section,
+						  ft_idx, ft_datasize))
+		    continue;
 		  ft_data = (bfd_byte *) bfd_malloc (ft_datasize);
 		  if (ft_data == NULL)
 		    continue;
@@ -1582,24 +1603,9 @@ pe_print_edata (bfd * abfd, void * vfile)
 		   _("\nThere is an export table, but the section containing it could not be found\n"));
 	  return true;
 	}
-      else if (!(section->flags & SEC_HAS_CONTENTS))
-	{
-	  fprintf (file,
-		   _("\nThere is an export table in %s, but that section has no contents\n"),
-		   section->name);
-	  return true;
-	}
 
       dataoff = addr - section->vma;
       datasize = extra->DataDirectory[PE_EXPORT_TABLE].Size;
-      if (dataoff > section->size
-	  || datasize > section->size - dataoff)
-	{
-	  fprintf (file,
-		   _("\nThere is an export table in %s, but it does not fit into that section\n"),
-		   section->name);
-	  return true;
-	}
     }
 
   /* PR 17512: Handle corrupt PE binaries.  */
@@ -1612,6 +1618,14 @@ pe_print_edata (bfd * abfd, void * vfile)
       return true;
     }
 
+  if (!get_contents_sanity_check (abfd, section, dataoff, datasize))
+    {
+      fprintf (file,
+	       _("\nThere is an export table in %s, but contents cannot be read\n"),
+	       section->name);
+      return true;
+    }
+
   /* xgettext:c-format */
   fprintf (file, _("\nThere is an export table in %s at 0x%lx\n"),
 	   section->name, (unsigned long) addr);
@@ -1622,7 +1636,10 @@ pe_print_edata (bfd * abfd, void * vfile)
 
   if (! bfd_get_section_contents (abfd, section, data,
 				  (file_ptr) dataoff, datasize))
-    return false;
+    {
+      free (data);
+      return false;
+    }
 
   /* Go get Export Directory Table.  */
   edt.export_flags   = bfd_get_32 (abfd, data +	 0);
@@ -3333,7 +3350,7 @@ rsrc_parse_entry (bfd *abfd,
   if (HighBitSet (val))
     {
       entry->is_dir = true;
-      entry->value.directory = bfd_malloc (sizeof * entry->value.directory);
+      entry->value.directory = bfd_malloc (sizeof (*entry->value.directory));
       if (entry->value.directory == NULL)
 	return dataend;
 
@@ -3344,12 +3361,12 @@ rsrc_parse_entry (bfd *abfd,
     }
 
   entry->is_dir = false;
-  entry->value.leaf = bfd_malloc (sizeof * entry->value.leaf);
+  entry->value.leaf = bfd_malloc (sizeof (*entry->value.leaf));
   if (entry->value.leaf == NULL)
     return dataend;
 
   data = datastart + val;
-  if (data < datastart || data >= dataend)
+  if (data < datastart || data + 12 > dataend)
     return dataend;
 
   addr = bfd_get_32 (abfd, data);
@@ -3357,6 +3374,8 @@ rsrc_parse_entry (bfd *abfd,
   entry->value.leaf->codepage = bfd_get_32 (abfd, data + 8);
   /* FIXME: We assume that the reserved field (data + 12) is OK.  */
 
+  if (size > dataend - datastart - (addr - rva_bias))
+    return dataend;
   entry->value.leaf->data = bfd_malloc (size);
   if (entry->value.leaf->data == NULL)
     return dataend;
@@ -3385,7 +3404,7 @@ rsrc_parse_entries (bfd *abfd,
       return highest_data;
     }
 
-  entry = bfd_malloc (sizeof * entry);
+  entry = bfd_malloc (sizeof (*entry));
   if (entry == NULL)
     return dataend;
 
@@ -3404,7 +3423,7 @@ rsrc_parse_entries (bfd *abfd,
 
       if (i)
 	{
-	  entry->next_entry = bfd_malloc (sizeof * entry);
+	  entry->next_entry = bfd_malloc (sizeof (*entry));
 	  entry = entry->next_entry;
 	  if (entry == NULL)
 	    return dataend;
@@ -4192,13 +4211,7 @@ rsrc_process_section (bfd * abfd,
 
   rva_bias = sec->vma - pe->pe_opthdr.ImageBase;
 
-  data = bfd_malloc (size);
-  if (data == NULL)
-    return;
-
-  datastart = data;
-
-  if (! bfd_get_section_contents (abfd, sec, data, 0, size))
+  if (! bfd_malloc_and_get_section (abfd, sec, &datastart))
     goto end;
 
   /* Step zero: Scan the input bfds looking for .rsrc sections and record
@@ -4210,7 +4223,8 @@ rsrc_process_section (bfd * abfd,
      at the end of a variable amount.  (It does not appear to be based upon
      the section alignment or the file alignment).  We need to skip any
      padding bytes when parsing the input .rsrc sections.  */
-  rsrc_sizes = bfd_malloc (max_num_input_rsrc * sizeof * rsrc_sizes);
+  data = datastart;
+  rsrc_sizes = bfd_malloc (max_num_input_rsrc * sizeof (*rsrc_sizes));
   if (rsrc_sizes == NULL)
     goto end;
 
@@ -4227,7 +4241,7 @@ rsrc_process_section (bfd * abfd,
 	    {
 	      max_num_input_rsrc += 10;
 	      rsrc_sizes = bfd_realloc (rsrc_sizes, max_num_input_rsrc
-					* sizeof * rsrc_sizes);
+					* sizeof (*rsrc_sizes));
 	      if (rsrc_sizes == NULL)
 		goto end;
 	    }
@@ -4280,7 +4294,7 @@ rsrc_process_section (bfd * abfd,
   data = datastart;
   rva_bias = sec->vma - pe->pe_opthdr.ImageBase;
 
-  type_tables = bfd_malloc (num_resource_sets * sizeof * type_tables);
+  type_tables = bfd_malloc (num_resource_sets * sizeof (*type_tables));
   if (type_tables == NULL)
     goto end;
 
@@ -4553,21 +4567,15 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
     if (sec)
       {
 	bfd_size_type x = sec->rawsize;
-	bfd_byte *tmp_data = NULL;
-
-	if (x)
-	  tmp_data = bfd_malloc (x);
+	bfd_byte *tmp_data;
 
-	if (tmp_data != NULL)
+	if (bfd_malloc_and_get_section (abfd, sec, &tmp_data))
 	  {
-	    if (bfd_get_section_contents (abfd, sec, tmp_data, 0, x))
-	      {
-		qsort (tmp_data,
-		       (size_t) (x / 12),
-		       12, sort_x64_pdata);
-		bfd_set_section_contents (pfinfo->output_bfd, sec,
-					  tmp_data, 0, x);
-	      }
+	    qsort (tmp_data,
+		   (size_t) (x / 12),
+		   12, sort_x64_pdata);
+	    bfd_set_section_contents (pfinfo->output_bfd, sec,
+				      tmp_data, 0, x);
 	    free (tmp_data);
 	  }
 	else

-- 
Alan Modra
Australia Development Lab, IBM