This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[PATCH] Fix BFD overflows (part 3)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: binutils at sources dot redhat dot com
- Date: Wed, 25 May 2005 19:03:35 +0200
- Subject: [PATCH] Fix BFD overflows (part 3)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
http://sources.redhat.com/ml/binutils/2005-05/msg00336.html
patch apparently broke both 64K+ section support and handling of
ELF objects with no section table. For both of these, e_shstrndx is
< e_shnum.
http://sources.redhat.com/ml/binutils/2005-05/msg00551.html
patch avoids crashes if a section group has valid sh_link,
but it doesn't point to a SHT_SYMTAB. But it still segfaults
if sh_link is plain invalid. To test, I have changed by hand sh_link
of one of the SHT_GROUP sections to insanely large value and strings
segfaulted.
The following patch fixes that and adds some more sanity checks
as well (e.g. i_shdr is usually larger than x_shdr, so although
we would fail if e_shnum * sizeof (x_shdr) overflows, we would
not fail if e_shnum * sizeof (i_shdr) overflows, so
amt = sizeof (*i_shdrp) * i_ehdrp->e_shnum;
i_shdrp = bfd_alloc (abfd, amt);
if (!i_shdrp)
goto got_no_match;
might not be big enough).
Ok to commit?
2005-05-25 Jakub Jelinek <jakub@redhat.com>
* elfcode.h (elf_object_p): Fail if e_shoff != 0, e_shnum == 0 and
first shdr has sh_size == 0. Fail if e_shnum is large to cause
arithmetic overflow when allocating the i_shdr array.
Sanity check sh_link and sh_info fields. Fix e_shstrndx sanity check.
--- bfd/elfcode.h.jj 2005-05-13 23:44:16.000000000 +0200
+++ bfd/elfcode.h 2005-05-25 18:41:36.000000000 +0200
@@ -632,7 +632,8 @@ elf_object_p (bfd *abfd)
if (i_ehdrp->e_shnum == SHN_UNDEF)
{
i_ehdrp->e_shnum = i_shdr.sh_size;
- if (i_ehdrp->e_shnum != i_shdr.sh_size)
+ if (i_ehdrp->e_shnum != i_shdr.sh_size
+ || i_ehdrp->e_shnum == 0)
goto got_wrong_format_error;
}
@@ -649,7 +650,8 @@ elf_object_p (bfd *abfd)
if (i_ehdrp->e_shnum != 1)
{
/* Check that we don't have a totally silly number of sections. */
- if (i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (x_shdr))
+ if (i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (x_shdr)
+ || i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (i_shdr))
goto got_wrong_format_error;
where += (i_ehdrp->e_shnum - 1) * sizeof (x_shdr);
@@ -670,10 +672,6 @@ elf_object_p (bfd *abfd)
}
}
- /* A further sanity check. */
- if (i_ehdrp->e_shstrndx >= i_ehdrp->e_shnum)
- goto got_wrong_format_error;
-
/* Allocate space for a copy of the section header table in
internal form. */
if (i_ehdrp->e_shnum != 0)
@@ -715,6 +713,20 @@ elf_object_p (bfd *abfd)
goto got_no_match;
elf_swap_shdr_in (abfd, &x_shdr, i_shdrp + shindex);
+ /* Sanity check sh_link and sh_info. */
+ if (i_shdrp[shindex].sh_link >= num_sec
+ || (i_shdrp[shindex].sh_link >= SHN_LORESERVE
+ && i_shdrp[shindex].sh_link <= SHN_HIRESERVE))
+ goto got_wrong_format_error;
+
+ if (((i_shdrp[shindex].sh_flags & SHF_INFO_LINK)
+ || i_shdrp[shindex].sh_type == SHT_RELA
+ || i_shdrp[shindex].sh_type == SHT_REL)
+ && (i_shdrp[shindex].sh_info >= num_sec
+ || (i_shdrp[shindex].sh_info >= SHN_LORESERVE
+ && i_shdrp[shindex].sh_info <= SHN_HIRESERVE)))
+ goto got_wrong_format_error;
+
/* If the section is loaded, but not page aligned, clear
D_PAGED. */
if (i_shdrp[shindex].sh_size != 0
@@ -727,6 +739,17 @@ elf_object_p (bfd *abfd)
}
}
+ /* A further sanity check. */
+ if (i_ehdrp->e_shnum != 0)
+ {
+ if (i_ehdrp->e_shstrndx >= elf_numsections (abfd)
+ || (i_ehdrp->e_shstrndx >= SHN_LORESERVE
+ && i_ehdrp->e_shstrndx <= SHN_HIRESERVE))
+ goto got_wrong_format_error;
+ }
+ else if (i_ehdrp->e_shstrndx != 0)
+ goto got_wrong_format_error;
+
/* Read in the program headers. */
if (i_ehdrp->e_phnum == 0)
elf_tdata (abfd)->phdr = NULL;
Jakub