[PATCH] array overrun, aout_link_input_section_ext

[PATCH] array overrun, aout_link_input_section_ext

[msnyder]

[-- Attachment #1: Type: text/plain, Size: 168 bytes --]

This one seems to be based on the assumption that BFD_ABORT aborts.
In this case, we'll get an error message, but we will still perform
an out of bounds array access.


[-- Attachment #2: r_type.txt --]
[-- Type: text/plain, Size: 734 bytes --]

2007-07-27  Michael Snyder  <msnyder@access-company.com>

	* aoutx.h (aout_link_input_section_ext): BFD_ASSERT does not
	abort -- bail out if r_type >= table size.

Index: aoutx.h
===================================================================
RCS file: /cvs/src/src/bfd/aoutx.h,v
retrieving revision 1.68
diff -p -r1.68 aoutx.h
*** aoutx.h	27 Jul 2007 19:04:39 -0000	1.68
--- aoutx.h	27 Jul 2007 20:41:48 -0000
*************** aout_link_input_section_ext (struct aout
*** 4287,4292 ****
--- 4287,4294 ----
        r_addend = GET_SWORD (input_bfd, rel->r_addend);
  
        BFD_ASSERT (r_type < TABLE_SIZE (howto_table_ext));
+       if (!(r_type < TABLE_SIZE (howto_table_ext)))
+ 	return FALSE;
  
        if (relocatable)
  	{

Re: [PATCH] array overrun, aout_link_input_section_ext

[Alan Modra]

On Fri, Jul 27, 2007 at 01:44:28PM -0700, msnyder@sonic.net wrote:
> 	* aoutx.h (aout_link_input_section_ext): BFD_ASSERT does not
> 	abort -- bail out if r_type >= table size.
> 
> Index: aoutx.h
> ===================================================================
> RCS file: /cvs/src/src/bfd/aoutx.h,v
> retrieving revision 1.68
> diff -p -r1.68 aoutx.h
> *** aoutx.h	27 Jul 2007 19:04:39 -0000	1.68
> --- aoutx.h	27 Jul 2007 20:41:48 -0000
> *************** aout_link_input_section_ext (struct aout
> *** 4287,4292 ****
> --- 4287,4294 ----
>         r_addend = GET_SWORD (input_bfd, rel->r_addend);
>   
>         BFD_ASSERT (r_type < TABLE_SIZE (howto_table_ext));
> +       if (!(r_type < TABLE_SIZE (howto_table_ext)))
> + 	return FALSE;
>   
>         if (relocatable)
>   	{

This shouldn't be an assert in the first place, since it would be
triggered by user input rather than a BFD programming error.  Also,
bfd_error should be set whenever returning false.

	* aoutx.h (swap_ext_reloc_in): Set howto to NULL for unknown
	r_type.
	(swap_std_reloc_in): Likewise.
	(aout_link_input_section_std): Likewise.  Return with an error
	on unexpected relocation type.
	(aout_link_input_section_ext): Likewise.

Index: bfd/aoutx.h
===================================================================
RCS file: /cvs/src/src/bfd/aoutx.h,v
retrieving revision 1.68
diff -u -p -r1.68 aoutx.h
--- bfd/aoutx.h	27 Jul 2007 19:04:39 -0000	1.68
+++ bfd/aoutx.h	1 Aug 2007 06:56:31 -0000
@@ -2162,7 +2162,10 @@ NAME (aout, swap_ext_reloc_in) (bfd *abf
 		>> RELOC_EXT_BITS_TYPE_SH_LITTLE);
     }
 
-  cache_ptr->howto =  howto_table_ext + r_type;
+  if (r_type < TABLE_SIZE (howto_table_ext))
+    cache_ptr->howto = howto_table_ext + r_type;
+  else
+    cache_ptr->howto = NULL;
 
   /* Base relative relocs are always against the symbol table,
      regardless of the setting of r_extern.  r_extern just reflects
@@ -2230,9 +2233,14 @@ NAME (aout, swap_std_reloc_in) (bfd *abf
 
   howto_idx = (r_length + 4 * r_pcrel + 8 * r_baserel
 	       + 16 * r_jmptable + 32 * r_relative);
-  BFD_ASSERT (howto_idx < TABLE_SIZE (howto_table_std));
-  cache_ptr->howto =  howto_table_std + howto_idx;
-  BFD_ASSERT (cache_ptr->howto->type != (unsigned int) -1);
+  if (howto_idx < TABLE_SIZE (howto_table_std))
+    {
+      cache_ptr->howto = howto_table_std + howto_idx;
+      if (cache_ptr->howto->type == (unsigned int) -1)
+	cache_ptr->howto = NULL;
+    }
+  else
+    cache_ptr->howto = NULL;
 
   /* Base relative relocs are always against the symbol table,
      regardless of the setting of r_extern.  r_extern just reflects
@@ -3963,11 +3971,21 @@ aout_link_input_section_std (struct aout
 
 	howto_idx = (r_length + 4 * r_pcrel + 8 * r_baserel
 		     + 16 * r_jmptable + 32 * r_relative);
-	BFD_ASSERT (howto_idx < TABLE_SIZE (howto_table_std));
-	howto = howto_table_std + howto_idx;
+	if (howto_idx < TABLE_SIZE (howto_table_std))
+	  howto = howto_table_std + howto_idx;
+	else
+	  howto = NULL;
       }
 #endif
 
+      if (howto == NULL)
+	{
+	  (*finfo->info->callbacks->einfo)
+	    (_("%P: %B: unexpected relocation type\n"), input_bfd);
+	  bfd_set_error (bfd_error_bad_value);
+	  return FALSE;
+	}
+
       if (relocatable)
 	{
 	  /* We are generating a relocatable output file, and must
@@ -4286,7 +4304,13 @@ aout_link_input_section_ext (struct aout
 
       r_addend = GET_SWORD (input_bfd, rel->r_addend);
 
-      BFD_ASSERT (r_type < TABLE_SIZE (howto_table_ext));
+      if (r_type >= TABLE_SIZE (howto_table_ext))
+	{
+	  (*finfo->info->callbacks->einfo)
+	    (_("%P: %B: unexpected relocation type\n"), input_bfd);
+	  bfd_set_error (bfd_error_bad_value);
+	  return FALSE;
+	}
 
       if (relocatable)
 	{

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] array overrun, aout_link_input_section_ext

[msnyder]

> This shouldn't be an assert in the first place, since it would be
> triggered by user input rather than a BFD programming error.  Also,
> bfd_error should be set whenever returning false.

Great, that takes care of the issue.  Thanks Alan.