Bugzilla component missing and another (minor) fuzzing-related bug report

Mark Wielaard mjw at redhat.com
Sat Jun 27 20:19:56 UTC 2015


On Sat, Jun 27, 2015 at 12:45:13PM +0200, Hanno Böck wrote:
> One reason you might want to fix such issues is that they could be used
> to cause memory exhaustion. E.g. you have a server that processes files
> and you send them specially crafted small files that will use up a lot
> of memory, but not that much that malloc failes.
> 
> Therefore imho it makes sense to add some sanity checks. Parsers should
> never accept any field sizes that are larger than the file itself.
> 
> This is probably not so much an issue in self-containing tools like
> elfutils. Honestly the biggest reason I report these is that asan
> complains about them and it makes fuzzing easier if they get fixed. But
> it's up to you. (Most other apps where I reported similar things fixed
> them)

The fix is indeed simple. We just have to switch the getting of data
(and detecing it is bogus) before allocating the memory. With that your
example gives:

src/nm: bogus.elf: entry size in section 2 `(null)' is not what we expect
src/nm: bogus.elf: INTERNAL ERROR 1207 (0.163): invalid data

And then exists before trying to allocate any memory.

Attached patch pushed to master.
Hope that helps. Looking forward to more fuzzing results :)

Thanks,

Mark
-------------- next part --------------
>From c08079a076420f67742be98d060500965eb22340 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw at redhat.com>
Date: Sat, 27 Jun 2015 22:07:01 +0200
Subject: [PATCH] nm: First call elf_getdata, then allocate memory.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This catches bogus data early before we might try to allocate giant
amounts of memory.

Reported-by: Hanno Böck <hanno at hboeck.de>
Signed-off-by: Mark Wielaard <mjw at redhat.com>
---
 src/ChangeLog |  4 ++++
 src/nm.c      | 12 ++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 7d5e001..50223a4 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2015-06-27  Mark Wielaard  <mjw at redhat.com>
+
+	* nm.c (show_symbols): First call elf_getdata, then allocate memory.
+
 2015-06-18  Mark Wielaard  <mjw at redhat.com>
 
 	* findtextrel.c (process_file): Free segments after use.
diff --git a/src/nm.c b/src/nm.c
index 7339506..15d9da4 100644
--- a/src/nm.c
+++ b/src/nm.c
@@ -1200,6 +1200,12 @@ show_symbols (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, Elf_Scn *xndxscn,
 	}
     }
 
+  /* Get the data of the section.  */
+  Elf_Data *data = elf_getdata (scn, NULL);
+  Elf_Data *xndxdata = elf_getdata (xndxscn, NULL);
+  if (data == NULL || (xndxscn != NULL && xndxdata == NULL))
+    INTERNAL_ERROR (fullname);
+
   /* Allocate the memory.
 
      XXX We can use a dirty trick here.  Since GElf_Sym == Elf64_Sym we
@@ -1211,12 +1217,6 @@ show_symbols (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, Elf_Scn *xndxscn,
   else
     sym_mem = (GElf_SymX *) xmalloc (nentries * sizeof (GElf_SymX));
 
-  /* Get the data of the section.  */
-  Elf_Data *data = elf_getdata (scn, NULL);
-  Elf_Data *xndxdata = elf_getdata (xndxscn, NULL);
-  if (data == NULL || (xndxscn != NULL && xndxdata == NULL))
-    INTERNAL_ERROR (fullname);
-
   /* Iterate over all symbols.  */
 #ifdef USE_DEMANGLE
   size_t demangle_buffer_len = 0;
-- 
2.4.3



More information about the elfutils-devel mailing list