Re: out-of-bounds read / crash in elfutils tools (readelf, nm, ...) with malformed file

[Mark Wielaard <mjw@redhat.com>]

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

On Tue, 2014-11-11 at 17:57 +0100, Mark Wielaard wrote:
> On Tue, Nov 11, 2014 at 02:57:05PM +0100, Hanno Böck wrote:
> > Am Tue, 11 Nov 2014 14:53:52 +0100
> > schrieb Mark Wielaard <mjw@redhat.com>:
> > 
> > > On Tue, 2014-11-11 at 14:40 +0100, Hanno Böck wrote:
> > > > I still get a bunch of crashers with correct LD_LIBRARY_PATH on
> > > > readelf -a with 32 bit compile (CFLAGS="-m32 -g"):
> > > > sig:11,hash:378b8b26
> > > > sig:11,hash:1aa8d351
> > > > sig:11,hash:872fe371
> > > > from attachment eu-readelf-crasher-hangs-2.tar.xz
> > > > 
> > > > and
> > > > id:000113,src:000000,op:flip32,pos:5474
> > > > id:000116,src:000000,op:flip32,pos:5554
> > > > from attachment 
> > > > /tmp/elfutils-nm-crasher.tar.xz
> > > 
> > > Could you attach or post those files somewhere?
> > 
> > These are all in attachments of previous mails in this thread:
> > 
> > eu-readelf-crasher-hangs-2.tar.xz
> > https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-November/004237.html
> > 
> > elfutils-nm-crasher.tar.xz
> > https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-November/004249.html
> 
> Aha, apparently I am unable to write correct overflow checks... sigh.
> 
> Please try the following:
> [...]

I pushed this now to master as attached.

Cheers,

Mark

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libelf-Fix-unsigned-overflow-check-in-elf_getdata.patch --]
[-- Type: text/x-patch, Size: 1456 bytes --]

From c50ddfca105a73f7567f3072831dcfbf49ad0567 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Thu, 13 Nov 2014 15:43:02 +0100
Subject: [PATCH] libelf: Fix unsigned overflow check in elf_getdata.

---
 libelf/ChangeLog     | 5 +++++
 libelf/elf_getdata.c | 5 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index a9d8c6f..45e220d 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2014-11-13  Mark Wielaard  <mjw@redhat.com>
+
+	* elf_getdata.c (__libelf_set_rawdata_wrlock): Fix unsigned overflow
+	check.
+
 2014-11-08  Mark Wielaard  <mjw@redhat.com>
 
 	* elf_begin.c (__libelf_next_arhdr_wrlock): Use mempcpy not __mempcpy.
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index 33d35d6..1ce1e23 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -245,9 +245,8 @@ __libelf_set_rawdata_wrlock (Elf_Scn *scn)
 	  /* First see whether the information in the section header is
 	     valid and it does not ask for too much.  Check for unsigned
 	     overflow.  */
-	  if (unlikely (offset + size > elf->maximum_size
-			|| (offset + size + elf->maximum_size
-			    < elf->maximum_size)))
+	  if (unlikely (offset > elf->maximum_size
+	      || elf->maximum_size - offset < size))
 	    {
 	      /* Something is wrong.  */
 	      __libelf_seterrno (ELF_E_INVALID_SECTION_HEADER);
-- 
1.8.3.1