Re: [PATCH] libdwfl: Check actually used ehsize, shentsize and phentsize in relocate.c

Re: [PATCH] libdwfl: Check actually used ehsize, shentsize and phentsize in relocate.c

[Mark Wielaard]

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

On Sun, 2014-12-07 at 23:45 +0100, Mark Wielaard wrote:
> In relocate_section we check relocation sections don't overlap any of the
> ELF headers. We should check against the actually used ehsize, shentsize
> and phentsize. Not the possibly bogus values in the file ehdr itself.
>
>  2014-12-07  Mark Wielaard  <mjw@redhat.com>
>  
> +	* relocate.c (relocate_section): Sanity check section overlap against
> +	actually used ehsize, shentsize and phentsize.

I pushed this to master now.

[PATCH] libdwfl: Check actually used ehsize, shentsize and phentsize in relocate.c

[Mark Wielaard]

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

In relocate_section we check relocation sections don't overlap any of the
ELF headers. We should check against the actually used ehsize, shentsize
and phentsize. Not the possibly bogus values in the file ehdr itself.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog  |  5 +++++
 libdwfl/relocate.c | 11 +++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 6eec018..b882f20 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,10 @@
 2014-12-07  Mark Wielaard  <mjw@redhat.com>
 
+	* relocate.c (relocate_section): Sanity check section overlap against
+	actually used ehsize, shentsize and phentsize.
+
+2014-12-07  Mark Wielaard  <mjw@redhat.com>
+
 	* offline.c (dwfl_offline_section_address): Assert shndx is not zero.
 	* relocate.c (__libdwfl_relocate_value): Don't relocate against
 	section zero.
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index fc8ae23..fd7dbd5 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -309,8 +309,9 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
      isn't illegal for ELF section data to overlap the header data,
      but updating the (relocation) data might corrupt the in-memory
      libelf headers causing strange corruptions or errors.  */
-  if (unlikely (shdr->sh_offset < ehdr->e_ehsize
-		|| tshdr->sh_offset < ehdr->e_ehsize))
+  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
+  if (unlikely (shdr->sh_offset < ehsize
+		|| tshdr->sh_offset < ehsize))
     return DWFL_E_BADELF;
 
   GElf_Off shdrs_start = ehdr->e_shoff;
@@ -318,7 +319,8 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
   if (elf_getshdrnum (relocated, &shnums) < 0)
     return DWFL_E_LIBELF;
   /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
-  GElf_Off shdrs_end = shdrs_start + shnums * ehdr->e_shentsize;
+  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
+  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
   if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
 		 && shdr->sh_offset < shdrs_end)
 		|| (shdrs_start < tshdr->sh_offset + tshdr->sh_size
@@ -332,7 +334,8 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
   if (phdrs_start != 0 && phnums != 0)
     {
       /* Overflows will have been checked by elf_getphdrnum/get|rawdata.  */
-      GElf_Off phdrs_end = phdrs_start + phnums * ehdr->e_phentsize;
+      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT);
+      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
       if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
 		     && shdr->sh_offset < phdrs_end)
 		    || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
-- 
1.9.3