PR25197, assertion fail coffgen.c

Message ID 20191119020340.GH13199@bubble.grove.modra.org
State New
Headers show
Series
  • PR25197, assertion fail coffgen.c
Related show

Commit Message

Alan Modra Nov. 19, 2019, 2:03 a.m.
The testcase in this PR triggered "BFD_ASSERT (p2->is_sym)" by
sneakily generating a C_FILE sym whose value pointed into auxents.
The fix then is in the last changed line of this patch, to check
p->is_sym as well as p->u.syment.n_sclass.  The other changes fix
various overflow checks that weren't as solid as they could be.

	PR 25197
	* coffgen.c (coff_find_nearest_line_with_names): Check that C_FILE
	u.syment.n_value does point at another C_FILE sym and not into
	some auxent that happens to look like a C_FILE.  Properly check
	for integer overflow and avoid possible pointer wrap-around.
	Simplify pr17512 checks.


-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/bfd/coffgen.c b/bfd/coffgen.c
index ba7bb5eaf4..7f26e18c45 100644
--- a/bfd/coffgen.c
+++ b/bfd/coffgen.c
@@ -1814,10 +1814,11 @@  coff_get_normalized_symtab (bfd *abfd)
   if (! _bfd_coff_get_external_symbols (abfd))
     return NULL;
 
-  size = obj_raw_syment_count (abfd) * sizeof (combined_entry_type);
+  size = obj_raw_syment_count (abfd);
   /* Check for integer overflow.  */
-  if (size < obj_raw_syment_count (abfd))
+  if (size > (bfd_size_type) -1 / sizeof (combined_entry_type))
     return NULL;
+  size *= sizeof (combined_entry_type);
   internal = (combined_entry_type *) bfd_zalloc (abfd, size);
   if (internal == NULL && size != 0)
     return NULL;
@@ -1844,29 +1845,20 @@  coff_get_normalized_symtab (bfd *abfd)
       symbol_ptr = internal_ptr;
       internal_ptr->is_sym = TRUE;
 
-      /* PR 17512: file: 1353-1166-0.004.  */
-      if (symbol_ptr->u.syment.n_sclass == C_FILE
-	  && symbol_ptr->u.syment.n_numaux > 0
-	  && raw_src + symesz + symbol_ptr->u.syment.n_numaux
-	  * symesz > raw_end)
-	{
-	  bfd_release (abfd, internal);
-	  return NULL;
-	}
-
       for (i = 0;
 	   i < symbol_ptr->u.syment.n_numaux;
 	   i++)
 	{
 	  internal_ptr++;
+	  raw_src += symesz;
+
 	  /* PR 17512: Prevent buffer overrun.  */
-	  if (internal_ptr >= internal_end)
+	  if (raw_src >= raw_end || internal_ptr >= internal_end)
 	    {
 	      bfd_release (abfd, internal);
 	      return NULL;
 	    }
 
-	  raw_src += symesz;
 	  bfd_coff_swap_aux_in (abfd, (void *) raw_src,
 				symbol_ptr->u.syment.n_type,
 				symbol_ptr->u.syment.n_sclass,
@@ -2408,13 +2400,16 @@  coff_find_nearest_line_with_names (bfd *abfd,
 	      maxdiff = offset + sec_vma - p2->u.syment.n_value;
 	    }
 
+	  if (p->u.syment.n_value >= cof->raw_syment_count)
+	    break;
+
 	  /* Avoid endless loops on erroneous files by ensuring that
 	     we always move forward in the file.  */
 	  if (p >= cof->raw_syments + p->u.syment.n_value)
 	    break;
 
 	  p = cof->raw_syments + p->u.syment.n_value;
-	  if (p > pend || p->u.syment.n_sclass != C_FILE)
+	  if (!p->is_sym || p->u.syment.n_sclass != C_FILE)
 	    break;
 	}
     }