This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Protect against integer overflow on shnum


If shnum is 0, the many "shnum - 1" would result in an overflow. Check it
for 0, and only subtract once, rather than on every usage.

Signed-off-by: Ulf Hermann <ulf.hermann@qt.io>
---
 libdwfl/ChangeLog              |  5 +++++
 libdwfl/dwfl_module_getdwarf.c | 18 ++++++++++--------
 src/ChangeLog                  |  4 ++++
 src/unstrip.c                  | 25 ++++++++++++++-----------
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 0a572ad..de73d79 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-20  Ulf Hermann  <ulf.hermann@qt.io>
 
+	* dwfl_module_getdwarf.c: Check shnum for 0 before subtracting from
+	it.
+
+2017-04-20  Ulf Hermann  <ulf.hermann@qt.io>
+
 	* libdwfl.h: Use __const_attribute__.
 
 2017-04-20  Ulf Hermann <ulf.hermann@qt.io>
diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
index 46caece..c8b839d 100644
--- a/libdwfl/dwfl_module_getdwarf.c
+++ b/libdwfl/dwfl_module_getdwarf.c
@@ -349,12 +349,14 @@ find_prelink_address_sync (Dwfl_Module *mod, struct dwfl_file *file)
 
   /* Since prelink does not store the zeroth section header in the undo
      section, it cannot support SHN_XINDEX encoding.  */
-  if (unlikely (shnum >= SHN_LORESERVE)
+  if (unlikely (shnum >= SHN_LORESERVE) || unlikely(shnum == 0)
       || unlikely (undodata->d_size != (src.d_size
 					+ phnum * phentsize
 					+ (shnum - 1) * shentsize)))
     return DWFL_E_BAD_PRELINK;
 
+  --shnum;
+
   /* We look at the allocated SHT_PROGBITS (or SHT_NOBITS) sections.  (Most
      every file will have some SHT_PROGBITS sections, but it's possible to
      have one with nothing but .bss, i.e. SHT_NOBITS.)  The special sections
@@ -431,12 +433,12 @@ find_prelink_address_sync (Dwfl_Module *mod, struct dwfl_file *file)
 
   src.d_buf += src.d_size;
   src.d_type = ELF_T_SHDR;
-  src.d_size = gelf_fsize (mod->main.elf, ELF_T_SHDR, shnum - 1, EV_CURRENT);
+  src.d_size = gelf_fsize (mod->main.elf, ELF_T_SHDR, shnum, EV_CURRENT);
 
   size_t shdr_size = class32 ? sizeof (Elf32_Shdr) : sizeof (Elf64_Shdr);
-  if (unlikely (shnum - 1  > SIZE_MAX / shdr_size))
+  if (unlikely (shnum > SIZE_MAX / shdr_size))
     return DWFL_E_NOMEM;
-  const size_t shdrs_bytes = (shnum - 1) * shdr_size;
+  const size_t shdrs_bytes = shnum * shdr_size;
   void *shdrs = malloc (shdrs_bytes);
   if (unlikely (shdrs == NULL))
     return DWFL_E_NOMEM;
@@ -488,16 +490,16 @@ find_prelink_address_sync (Dwfl_Module *mod, struct dwfl_file *file)
       highest = 0;
       if (class32)
 	{
-	  Elf32_Shdr (*s32)[shnum - 1] = shdrs;
-	  for (size_t i = 0; i < shnum - 1; ++i)
+	  Elf32_Shdr (*s32)[shnum] = shdrs;
+	  for (size_t i = 0; i < shnum; ++i)
 	    consider_shdr (undo_interp, (*s32)[i].sh_type,
 			   (*s32)[i].sh_flags, (*s32)[i].sh_addr,
 			   (*s32)[i].sh_size, &highest);
 	}
       else
 	{
-	  Elf64_Shdr (*s64)[shnum - 1] = shdrs;
-	  for (size_t i = 0; i < shnum - 1; ++i)
+	  Elf64_Shdr (*s64)[shnum] = shdrs;
+	  for (size_t i = 0; i < shnum; ++i)
 	    consider_shdr (undo_interp, (*s64)[i].sh_type,
 			   (*s64)[i].sh_flags, (*s64)[i].sh_addr,
 			   (*s64)[i].sh_size, &highest);
diff --git a/src/ChangeLog b/src/ChangeLog
index e0a591e..1521d80 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-20  Ulf Hermann  <ulf.hermann@qt.io>
+
+	* unstrip.c: Check shnum for 0 before subtracting from it.
+
 2017-04-20  Ulf Hermann <ulf.hermann@qt.io>
 
 	* readelf.c: Replace YESSTR and NOSTR with gettext ("yes") and
diff --git a/src/unstrip.c b/src/unstrip.c
index 6e57a6b..5074909 100644
--- a/src/unstrip.c
+++ b/src/unstrip.c
@@ -1027,21 +1027,24 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data *debug_shstrtab,
 	  shnum = ehdr.e64.e_shnum;
 	}
 
+      bool class32 = ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32;
+      size_t shsize = class32 ? sizeof (Elf32_Shdr) : sizeof (Elf64_Shdr);
+      if (unlikely (shnum == 0 || shnum > SIZE_MAX / shsize + 1))
+	error (EXIT_FAILURE, 0, _("overflow with shnum = %zu in '%s' section"),
+	       (size_t) shnum, ".gnu.prelink_undo");
+
+      --shnum;
+
       size_t phsize = gelf_fsize (main, ELF_T_PHDR, phnum, EV_CURRENT);
       src.d_buf += src.d_size + phsize;
-      src.d_size = gelf_fsize (main, ELF_T_SHDR, shnum - 1, EV_CURRENT);
+      src.d_size = gelf_fsize (main, ELF_T_SHDR, shnum, EV_CURRENT);
       src.d_type = ELF_T_SHDR;
       if ((size_t) (src.d_buf - undodata->d_buf) > undodata->d_size
 	  || undodata->d_size - (src.d_buf - undodata->d_buf) != src.d_size)
 	error (EXIT_FAILURE, 0, _("invalid contents in '%s' section"),
 	       ".gnu.prelink_undo");
 
-      bool class32 = ehdr.e32.e_ident[EI_CLASS] == ELFCLASS32;
-      size_t shsize = class32 ? sizeof (Elf32_Shdr) : sizeof (Elf64_Shdr);
-      if (unlikely ((shnum - 1) > SIZE_MAX / shsize))
-	error (EXIT_FAILURE, 0, _("overflow with shnum = %zu in '%s' section"),
-	       (size_t) shnum, ".gnu.prelink_undo");
-      const size_t shdr_bytes = (shnum - 1) * shsize;
+      const size_t shdr_bytes = shnum * shsize;
       void *shdr = xmalloc (shdr_bytes);
       dst.d_buf = shdr;
       dst.d_size = shdr_bytes;
@@ -1049,12 +1052,12 @@ find_alloc_sections_prelink (Elf *debug, Elf_Data *debug_shstrtab,
 				main_ehdr->e_ident[EI_DATA]) != NULL,
 		 _("cannot read '.gnu.prelink_undo' section: %s"));
 
-      undo_sections = xmalloc ((shnum - 1) * sizeof undo_sections[0]);
-      for (size_t i = 0; i < shnum - 1; ++i)
+      undo_sections = xmalloc (shnum * sizeof undo_sections[0]);
+      for (size_t i = 0; i < shnum; ++i)
 	{
 	  struct section *sec = &undo_sections[undo_nalloc];
-	  Elf32_Shdr (*s32)[shnum - 1] = shdr;
-	  Elf64_Shdr (*s64)[shnum - 1] = shdr;
+	  Elf32_Shdr (*s32)[shnum] = shdr;
+	  Elf64_Shdr (*s64)[shnum] = shdr;
 	  if (class32)
 	    {
 #define COPY(field) sec->shdr.field = (*s32)[i].field
-- 
2.1.4


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