Re: [PATCH] libelf: Sanity check offset and size before trying to malloc and read data.

Re: [PATCH] libelf: Sanity check offset and size before trying to malloc and read data.

[Mark Wielaard]

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

On Fri, 2016-10-21 at 15:24 +0200, Mark Wielaard wrote:
> Bad sh_off or sh_size could trigger a bad malloc or read. Sanity check
> the header values first before trying to malloc a huge buffer or reading
> any data that will certainly fail.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1387584

I didn't hear back from the original reporter whether this really solved
their problem. But the change does look correct and desirable. So I have
pushed this to master now.

[PATCH] libelf: Sanity check offset and size before trying to malloc and read data.

[Mark Wielaard]

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

Bad sh_off or sh_size could trigger a bad malloc or read. Sanity check
the header values first before trying to malloc a huge buffer or reading
any data that will certainly fail.

https://bugzilla.redhat.com/show_bug.cgi?id=1387584

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libelf/ChangeLog     |  5 +++++
 libelf/elf_getdata.c | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 35af786..89546ab 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2016-10-21  Mark Wielaard  <mjw@redhat.com>
+
+	* elf_getdata.c (__libelf_set_rawdata_wrlock): Sanity check
+	offset and size before trying to malloc and read data.
+
 2015-10-11  Akihiko Odaki  <akihiko.odaki.4i@stu.hosei.ac.jp>
 
 	* elf_getarsym.c (elf_getarsym): Open code rawmemchr when not
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index d1fafbf..97c503b 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -312,6 +312,17 @@ __libelf_set_rawdata_wrlock (Elf_Scn *scn)
 	}
       else if (likely (elf->fildes != -1))
 	{
+	  /* 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 > elf->maximum_size
+			|| elf->maximum_size - offset < size))
+	    {
+	      /* Something is wrong.  */
+	      __libelf_seterrno (ELF_E_INVALID_SECTION_HEADER);
+	      return 1;
+	    }
+
 	  /* We have to read the data from the file.  Allocate the needed
 	     memory.  */
 	  scn->rawdata_base = scn->rawdata.d.d_buf
-- 
1.8.3.1