This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
[PATCH] Protect against integer overflow on shnum
- From: Ulf Hermann <ulf dot hermann at qt dot io>
- To: <elfutils-devel at sourceware dot org>
- Date: Thu, 20 Apr 2017 16:04:54 +0200
- Subject: [PATCH] Protect against integer overflow on shnum
- Authentication-results: sourceware.org; auth=none
- Authentication-results: sourceware.org; dkim=none (message not signed) header.d=none;sourceware.org; dmarc=none action=none header.from=qt.io;
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qtcompany.onmicrosoft.com; s=selector1-qt-io; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=P+jOHLnLiSbOAie3SENW1FXHSTT2FzRvGf+kpNnc6jk=; b=hxTgCaQMTvNr/rp6u1blI0BuN71ucLhrx/jEH006y1pCUX5Yg+ZoJyypJb8KtibCkYbJgTNA9tCWQOrRGSQj5vjJa+P3qpOOaqjKBfCFYHZICeUwcogFqQp+WcAbWEXeUGVvbAghkkefUgmM3A7G7H07NKdE3KBtxrnfrhsSiTA=
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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