[PATCH] Further bfd robustification

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Further bfd robustification

Jakub Jelinek
Hi!

Avoid crashes with bogus sh_entsize and 3 COFF failures.
I'm afraid there are hundreds of places in the COFF etc. code that needs
protection though.

2005-06-14  Jakub Jelinek  <[hidden email]>

        * elf.c (bfd_section_from_shdr): Fail if sh_entsize is bogus for
        symbol, relocation, group or versym sections.

        * coffcode.h (coff_slurp_reloc_table): Don't crash if native_relocs
        is NULL.
        * peXXigen.c (pe_print_idata): Don't crash if dll_name or start_address
        doesn't point into the section.

--- bfd/elf.c.jj 2005-06-14 13:08:41.000000000 +0200
+++ bfd/elf.c 2005-06-14 14:04:11.000000000 +0200
@@ -1803,7 +1803,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
       if (elf_onesymtab (abfd) == shindex)
  return TRUE;
 
-      BFD_ASSERT (hdr->sh_entsize == bed->s->sizeof_sym);
+      if (hdr->sh_entsize != bed->s->sizeof_sym)
+ return FALSE;
       BFD_ASSERT (elf_onesymtab (abfd) == 0);
       elf_onesymtab (abfd) = shindex;
       elf_tdata (abfd)->symtab_hdr = *hdr;
@@ -1854,7 +1855,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
       if (elf_dynsymtab (abfd) == shindex)
  return TRUE;
 
-      BFD_ASSERT (hdr->sh_entsize == bed->s->sizeof_sym);
+      if (hdr->sh_entsize != bed->s->sizeof_sym)
+ return FALSE;
       BFD_ASSERT (elf_dynsymtab (abfd) == 0);
       elf_dynsymtab (abfd) = shindex;
       elf_tdata (abfd)->dynsymtab_hdr = *hdr;
@@ -1938,6 +1940,10 @@ bfd_section_from_shdr (bfd *abfd, unsign
  Elf_Internal_Shdr *hdr2;
  unsigned int num_sec = elf_numsections (abfd);
 
+ if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
+ ? bed->s->sizeof_rel : bed->s->sizeof_rela))
+  return FALSE;
+
  /* Check for a bogus link to avoid crashing.  */
  if ((hdr->sh_link >= SHN_LORESERVE && hdr->sh_link <= SHN_HIRESERVE)
     || hdr->sh_link >= num_sec)
@@ -1996,10 +2002,10 @@ bfd_section_from_shdr (bfd *abfd, unsign
   return _bfd_elf_make_section_from_shdr (abfd, hdr, name,
   shindex);
 
-        /* Prevent endless recursion on broken objects.  */
-        if (elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_REL
-            || elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_RELA)
-          return FALSE;
+ /* Prevent endless recursion on broken objects.  */
+ if (elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_REL
+    || elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_RELA)
+  return FALSE;
  if (! bfd_section_from_shdr (abfd, hdr->sh_info))
   return FALSE;
  target_sect = bfd_section_from_elf_index (abfd, hdr->sh_info);
@@ -2039,6 +2045,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
       break;
 
     case SHT_GNU_versym:
+      if (hdr->sh_entsize != 2)
+ return FALSE;
       elf_dynversym (abfd) = shindex;
       elf_tdata (abfd)->dynversym_hdr = *hdr;
       return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex);
@@ -2057,6 +2065,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
       /* We need a BFD section for objcopy and relocatable linking,
  and it's handy to have the signature available as the section
  name.  */
+      if (hdr->sh_entsize != 4)
+ return FALSE;
       name = group_signature (abfd, hdr);
       if (name == NULL)
  return FALSE;
--- bfd/coffcode.h.jj 2005-06-14 12:12:03.000000000 +0200
+++ bfd/coffcode.h 2005-06-14 14:17:22.000000000 +0200
@@ -4830,7 +4830,7 @@ coff_slurp_reloc_table (bfd * abfd, sec_
   amt = (bfd_size_type) asect->reloc_count * sizeof (arelent);
   reloc_cache = bfd_alloc (abfd, amt);
 
-  if (reloc_cache == NULL)
+  if (reloc_cache == NULL || native_relocs == NULL)
     return FALSE;
 
   for (idx = 0; idx < asect->reloc_count; idx++)
--- bfd/peXXigen.c.jj 2005-05-13 23:44:20.000000000 +0200
+++ bfd/peXXigen.c 2005-06-14 14:47:21.000000000 +0200
@@ -1103,7 +1103,7 @@ pe_print_idata (bfd * abfd, void * vfile
       bfd_vma toc_address;
       bfd_vma start_address;
       bfd_byte *data;
-      int offset;
+      bfd_vma offset;
 
       if (!bfd_malloc_and_get_section (abfd, rel_section, &data))
  {
@@ -1114,6 +1114,13 @@ pe_print_idata (bfd * abfd, void * vfile
 
       offset = abfd->start_address - rel_section->vma;
 
+      if (offset >= rel_section->size || offset + 8 > rel_section->size)
+        {
+          if (data != NULL)
+            free (data);
+          return FALSE;
+        }
+
       start_address = bfd_get_32 (abfd, data + offset);
       loadable_toc_address = bfd_get_32 (abfd, data + offset + 4);
       toc_address = loadable_toc_address - 32768;
@@ -1182,6 +1189,9 @@ pe_print_idata (bfd * abfd, void * vfile
       if (hint_addr == 0 && first_thunk == 0)
  break;
 
+      if (dll_name - adj >= section->size)
+        break;
+
       dll = (char *) data + dll_name - adj;
       fprintf (file, _("\n\tDLL Name: %s\n"), dll);
 

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Further bfd robustification

Nick Clifton
Hi Jakub,

> 2005-06-14  Jakub Jelinek  <[hidden email]>
>
> * elf.c (bfd_section_from_shdr): Fail if sh_entsize is bogus for
> symbol, relocation, group or versym sections.
>
> * coffcode.h (coff_slurp_reloc_table): Don't crash if native_relocs
> is NULL.
> * peXXigen.c (pe_print_idata): Don't crash if dll_name or start_address
> doesn't point into the section.

Just one small issue:

>      case SHT_GNU_versym:
> +      if (hdr->sh_entsize != 2)
> + return FALSE;

> @@ -2057,6 +2065,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
>        /* We need a BFD section for objcopy and relocatable linking,
>   and it's handy to have the signature available as the section
>   name.  */
> +      if (hdr->sh_entsize != 4)
> + return FALSE;

I really think that these magic values ought to be defined macro
constants declared in a header file somewhere (include/elf/common.h ?).

Cheers
   Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Further bfd robustification

Jakub Jelinek
On Thu, Jun 16, 2005 at 10:42:00AM +0100, Nick Clifton wrote:
> Just one small issue:
>
> >     case SHT_GNU_versym:
> >+      if (hdr->sh_entsize != 2)
> >+ return FALSE;

Ok, how about sizeof (Elf_External_Versym) in that case?

> >@@ -2057,6 +2065,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
> >       /* We need a BFD section for objcopy and relocatable linking,
> > and it's handy to have the signature available as the section
> > name.  */
> >+      if (hdr->sh_entsize != 4)
> >+ return FALSE;
>
> I really think that these magic values ought to be defined macro
> constants declared in a header file somewhere (include/elf/common.h ?).

4 is used in many places for size of SHT_GROUP entries, in elf.c and
elsewhere.  The ELF standard mandates that sh_entsize is 4 for that section.
But if you don't like numbers, we could perhaps add
Elf_External_Groupidx or something like that to include/elf/external.h
and use sizeof (Elf_External_Groupidx) there.

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Further bfd robustification

Nick Clifton
Hi Jakub,

>>>    case SHT_GNU_versym:
>>>+      if (hdr->sh_entsize != 2)
>>>+ return FALSE;

> Ok, how about sizeof (Elf_External_Versym) in that case?

Yes please.


>>>@@ -2057,6 +2065,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
>>>      /* We need a BFD section for objcopy and relocatable linking,
>>> and it's handy to have the signature available as the section
>>> name.  */
>>>+      if (hdr->sh_entsize != 4)
>>>+ return FALSE;
>>
>>I really think that these magic values ought to be defined macro
>>constants declared in a header file somewhere (include/elf/common.h ?).
>
>
> 4 is used in many places for size of SHT_GROUP entries, in elf.c and
> elsewhere.  The ELF standard mandates that sh_entsize is 4 for that section.
> But if you don't like numbers, we could perhaps add
> Elf_External_Groupidx or something like that to include/elf/external.h
> and use sizeof (Elf_External_Groupidx) there.

Well if the standard mandates 4 then we can just add a #define to
external.h.  There is no need for a sizeof() unless we are dealing with
a structure of some kind.

Cheers
   Nick