This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Bogus code in coffgen.c?


Hi H.J.

> -  if (bfd_coff_bad_format_hook (abfd, &internal_f) == false)
> +  if (bfd_coff_bad_format_hook (abfd, &internal_f) == false
> +      || (internal_f.f_opthdr != 0 && internal_f.f_opthdr != aoutsz))

This does not look right.  According to Ian:

> > For a normal COFF target, f_opthdr should be either 0 or aoutsz.
> > XCOFF is an irritating exception: XCOFF defines a large and a small
> > aout header (I believe the small header is used for an object file
> > while the large header is used for an executable), so for XCOFF you
> > have to pay attention to f_opthdr, and not read more than that.  But
> > you still want to allocate aoutsz bytes.  because that is what
> > swap_aouthdr_in and friends expect, even for a small XCOFF header.

So for an XCOFF object file the small header will be used (where
internal_f.f_opthdr is less than aoutsz) and your code would reject
this.

How about this alternative ?

Cheers
        Nick

2001-11-02  H.J. Lu  (hjl@gnu.org) & Nick Clifton <nickc@redhat.com>

	* coffgen.c (coff_object_p): Reject headers with corrupt
	lengths in their internal_f.f_opthdr field.

Index: bfd/coffgen.c
===================================================================
RCS file: /cvs/src/src/bfd/coffgen.c,v
retrieving revision 1.27
diff -c -p -r1.27 coffgen.c
*** coffgen.c	2001/10/10 12:08:27	1.27
--- coffgen.c	2001/11/02 16:06:23
*************** coff_object_p (abfd)
*** 291,299 ****
      {
        PTR opthdr;
  
        opthdr = bfd_alloc (abfd, aoutsz);
        if (opthdr == NULL)
! 	return 0;;
        if (bfd_bread (opthdr, (bfd_size_type) internal_f.f_opthdr, abfd)
  	  != internal_f.f_opthdr)
  	{
--- 291,313 ----
      {
        PTR opthdr;
  
+       /* The XCOFF format has two sizes for the f_opthdr.  A small size
+ 	 (less than aoutsz) used in object files and a large size (equal
+ 	 to aoutsz) in executables.  The bfd_coff_swap_aouthdr_in expects
+ 	 this header to be aoutsz bytes in length, so we use that value
+ 	 in the call to bfd_alloc below, but we must be careful to only
+ 	 read in f_opthdr bytes in the call to bfd_bread.  We should also
+ 	 attempt to catch corrupt or non-COFF binaries with a strange
+        value for f_opthdr.  */
+       if (internal_f.f_opthdr > aoutsz)
+ 	{
+ 	  bfd_set_error (bfd_error_wrong_format);
+ 	  return 0;
+ 	}
+ 
        opthdr = bfd_alloc (abfd, aoutsz);
        if (opthdr == NULL)
! 	return 0;
        if (bfd_bread (opthdr, (bfd_size_type) internal_f.f_opthdr, abfd)
  	  != internal_f.f_opthdr)
  	{


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]