[Bug backends/24075] Program Crash due to buffer over-read in ebl_object_note function in eblobjnote.c in libebl.

2019-01-16 Thread mark at klomp dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=24075

Mark Wielaard  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 CC||mark at klomp dot org
 Resolution|--- |FIXED
Summary|Program Crash due to Wild   |Program Crash due to buffer
   |pointer Deference in|over-read in
   |ebl_object_note function in |ebl_object_note function in
   |eblobjnote.c in libebl. |eblobjnote.c in libebl.

--- Comment #3 from Mark Wielaard  ---
(In reply to wcventure from comment #0)

> Our fuzzer caught Pointer Deference problem in eu-readelf of the latest
> elfutils-0.174 code base, this inputs will cause the segment faults and I
> have confirmed them with address sanitizer too. Please use the "./eu-readelf
> -a $POC"to reproduce the bug. If you have any questions, please let me know.

This code was introduced in 0.175 and not present in 0.174.
Confirmed by running the reproducer under valgrind.

> This problem is in the code as fllow, it seem like a use-after-fee problem.
> 
> > size_t i;
> > for (i = 0; i < prop.pr_datasz - 1; i++)
> > printf ("%02" PRIx8 " ", (uint8_t) desc[i]);

Yes, this over-reads the buffer because pr_datasz isn't checked.
Fixed as follows:

commit 012018907ca05eb0ab51d424a596ef38fc87cae1
Author: Mark Wielaard 
Date:   Wed Jan 16 11:57:35 2019 +0100

libebl: Check GNU property note pr_datasz fits inside note description.

Before printing the data values, make sure pr_datasz doesn't go beyond
the end of the note description data.

https://sourceware.org/bugzilla/show_bug.cgi?id=24075

Signed-off-by: Mark Wielaard 

diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index 0174f33..77c2274 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,7 @@
+2019-01-16  Mark Wielaard  
+
+   * eblobjnte.c (ebl_object_note): Check pr_datasz isn't too large.
+
 2018-12-02  Mark Wielaard  

* eblobjnte.c (ebl_object_note): For GNU_PROPERTY_STACK_SIZE use
diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
index c19ea37..9094715 100644
--- a/libebl/eblobjnote.c
+++ b/libebl/eblobjnote.c
@@ -350,6 +350,13 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char
*name, uint32_t type,
  desc += 8;
  descsz -= 8;

+ if (prop.pr_datasz > descsz)
+   {
+ printf ("BAD property datasz: %" PRId32 "\n",
+ prop.pr_datasz);
+ return;
+   }
+
  int elfclass = gelf_getclass (ebl->elf);
  char *elfident = elf_getident (ebl->elf, NULL);
  GElf_Ehdr ehdr;

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug backends/24084] Negative-size-param when when calling memcpy function in elf_cvt_note function in libelf

2019-01-16 Thread mark at klomp dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=24084

Mark Wielaard  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 CC||mark at klomp dot org
 Resolution|--- |FIXED

--- Comment #1 from Mark Wielaard  ---
(In reply to wcventure from comment #0)
> Negative-size-param when calling memcpy function in elf_cvt_note function in
> libelf the latest elfutils-0.174 code base, this inputs will cause the
> segment faults and I have confirmed them with address sanitizer too. 

Nice find. Replicated under valgrind with the reproducer.
The root cause is a wrong overflow check.
The code wanted to make sure we had at least enough room for the header,
but got the size of the header wrong. It had hardcoded the size as 8 bytes,
but it should have been 12 bytes.

Fixed as follows:

commit e65d91d21cb09d83b001fef9435e576ba447db32
Author: Mark Wielaard 
Date:   Wed Jan 16 12:25:57 2019 +0100

libelf: Correct overflow check in note_xlate.

We want to make sure the note_len doesn't overflow and becomes shorter
than the note header. But the namesz and descsz checks got the note header
size wrong). Replace the wrong constant (8) with a sizeof cvt_Nhdr (12).

https://sourceware.org/bugzilla/show_bug.cgi?id=24084

Signed-off-by: Mark Wielaard 

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 5923c85..5783f0c 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-16  Mark Wielaard  
+
+   * note_xlate.h (elf_cvt_note): Check n_namesz and n_descsz don't
+   overflow note_len into note header.
+
 2018-11-17  Mark Wielaard  

* elf32_updatefile.c (updatemmap): Make sure to call convert
diff --git a/libelf/note_xlate.h b/libelf/note_xlate.h
index 9bdc3e2..bc9950f 100644
--- a/libelf/note_xlate.h
+++ b/libelf/note_xlate.h
@@ -46,13 +46,13 @@ elf_cvt_note (void *dest, const void *src, size_t len, int
encode,
   /* desc needs to be aligned.  */
   note_len += n->n_namesz;
   note_len = nhdr8 ? NOTE_ALIGN8 (note_len) : NOTE_ALIGN4 (note_len);
-  if (note_len > len || note_len < 8)
+  if (note_len > len || note_len < sizeof *n)
break;

   /* data as a whole needs to be aligned.  */
   note_len += n->n_descsz;
   note_len = nhdr8 ? NOTE_ALIGN8 (note_len) : NOTE_ALIGN4 (note_len);
-  if (note_len > len || note_len < 8)
+  if (note_len > len || note_len < sizeof *n)
break;

   /* Copy or skip the note data.  */

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[PATCH] libebl: Check NT_PLATFORM core notes contain a zero terminated string.

2019-01-16 Thread Mark Wielaard
Most strings in core notes are fixed size. But NT_PLATFORM contains just
a variable length string. Check that it is actually zero terminated
before passing to readelf to print.

https://sourceware.org/bugzilla/show_bug.cgi?id=24089

Signed-off-by: Mark Wielaard 
---
 libdwfl/ChangeLog   |  5 +
 libdwfl/linux-core-attach.c |  9 +
 libebl/ChangeLog|  6 ++
 libebl/eblcorenote.c| 39 +++
 libebl/libebl.h |  3 ++-
 src/ChangeLog   |  4 
 src/readelf.c   |  2 +-
 7 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index b96cebf..c295fa7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-16  Mark Wielaard  
+
+   * linux-core-attach.c (core_next_thread): Pass desc to ebl_core_note.
+   (core_set_initial_registers): Likewise.
+
 2018-10-23  Mark Wielaard  
 
* relocate.c (relocate_section): Only sanity check mmapped Elf files
diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
index 6c99b9e..c0f1b0d 100644
--- a/libdwfl/linux-core-attach.c
+++ b/libdwfl/linux-core-attach.c
@@ -137,7 +137,7 @@ core_next_thread (Dwfl *dwfl __attribute__ ((unused)), void 
*dwfl_arg,
   const Ebl_Register_Location *reglocs;
   size_t nitems;
   const Ebl_Core_Item *items;
-  if (! ebl_core_note (core_arg->ebl, &nhdr, name,
+  if (! ebl_core_note (core_arg->ebl, &nhdr, name, desc,
   ®s_offset, &nregloc, ®locs, &nitems, &items))
{
  /* This note may be just not recognized, skip it.  */
@@ -191,8 +191,9 @@ core_set_initial_registers (Dwfl_Thread *thread, void 
*thread_arg_voidp)
   const Ebl_Register_Location *reglocs;
   size_t nitems;
   const Ebl_Core_Item *items;
-  int core_note_err = ebl_core_note (core_arg->ebl, &nhdr, name, ®s_offset,
-&nregloc, ®locs, &nitems, &items);
+  int core_note_err = ebl_core_note (core_arg->ebl, &nhdr, name, desc,
+®s_offset, &nregloc, ®locs,
+&nitems, &items);
   /* __libdwfl_attach_state_for_core already verified the note is there.  */
   assert (core_note_err != 0);
   assert (nhdr.n_type == NT_PRSTATUS);
@@ -383,7 +384,7 @@ dwfl_core_file_attach (Dwfl *dwfl, Elf *core)
   const Ebl_Register_Location *reglocs;
   size_t nitems;
   const Ebl_Core_Item *items;
-  if (! ebl_core_note (ebl, &nhdr, name,
+  if (! ebl_core_note (ebl, &nhdr, name, desc,
   ®s_offset, &nregloc, ®locs, &nitems, &items))
{
  /* This note may be just not recognized, skip it.  */
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index 77c2274..9cdf899 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,5 +1,11 @@
 2019-01-16  Mark Wielaard  
 
+   * libebl.h (ebl_core_note): Add desc as argument.
+   * eblcorenote.c (ebl_core_note): Take desc as an argument, check
+   it contains a zero terminated string if it is an NT_PLATFORM note.
+
+2019-01-16  Mark Wielaard  
+
* eblobjnte.c (ebl_object_note): Check pr_datasz isn't too large.
 
 2018-12-02  Mark Wielaard  
diff --git a/libebl/eblcorenote.c b/libebl/eblcorenote.c
index 783f981..7fab397 100644
--- a/libebl/eblcorenote.c
+++ b/libebl/eblcorenote.c
@@ -36,11 +36,13 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 
 int
 ebl_core_note (Ebl *ebl, const GElf_Nhdr *nhdr, const char *name,
+  const char *desc,
   GElf_Word *regs_offset, size_t *nregloc,
   const Ebl_Register_Location **reglocs, size_t *nitems,
   const Ebl_Core_Item **items)
@@ -51,28 +53,25 @@ ebl_core_note (Ebl *ebl, const GElf_Nhdr *nhdr, const char 
*name,
 {
   /* The machine specific function did not know this type.  */
 
-  *regs_offset = 0;
-  *nregloc = 0;
-  *reglocs = NULL;
-  switch (nhdr->n_type)
+  /* NT_PLATFORM is kind of special since it needs a zero terminated
+ string (other notes often have a fixed size string).  */
+  static const Ebl_Core_Item platform[] =
{
-#define ITEMS(type, table) \
- case type:\
-   *items = table; \
-   *nitems = sizeof table / sizeof table[0];   \
-   result = 1; \
-   break
+ {
+   .name = "Platform",
+   .type = ELF_T_BYTE, .count = 0, .format = 's'
+ }
+   };
 
- static const Ebl_Core_Item platform[] =
-   {
- {
-   .name = "Platform",
-   .type = ELF_T_BYTE, .count = 0, .format = 's'
- }
-   };
- ITEMS (NT_PLATFORM, platform);
-
-#undef ITEMS
+  if (nhdr->n_type == NT_PLATFORM
+ && memchr (desc

[Bug tools/24089] NT_PLATFORM core file note should be a zero terminated string

2019-01-16 Thread mark at klomp dot org
https://sourceware.org/bugzilla/show_bug.cgi?id=24089

Mark Wielaard  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2019-01-16
 CC||mark at klomp dot org
  Component|libelf  |tools
Summary|A Heap-buffer-overflow  |NT_PLATFORM core file note
   |problem was discovered in   |should be a zero terminated
   |the function elf32_xlatetom |string
   |in elf32_xlatetom.c in  |
   |libelf  |
 Ever confirmed|0   |1

--- Comment #2 from Mark Wielaard  ---
(In reply to wcventure from comment #0)
> A Heap-buffer-overflow problem was discovered in the function elf32_xlatetom
> in elf32_xlatetom.c in libelf, as distributed in ELFutils 0.147. A crafted
> ELF input can cause segment faults and I have confirmed them with address
> sanitizer too.

Interesting. The same can be found running the reproducer under valgrind.
The issue is that when eu-readelf -n tries to print the values of a core file
ELF note and sees a NT_PLATFORM type, it doesn't check that the value is a
properly zero terminated string.

The simplest solution is to just not recognize such corrupt core file notes in
ebl_core_note:
https://sourceware.org/ml/elfutils-devel/2019-q1/msg00049.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [PATCH] Skip run-readelf-compressed.sh test if built without bzip2

2019-01-16 Thread Mark Wielaard
On Mon, 2019-01-14 at 08:27 +, Ulf Hermann wrote:
> > Added a ChangeLog and pushed to master.
> > Please don't sent patches with base64 encoding.
> > That make it really hard to apply them with git am.
> 
> Thanks, and sorry. As the message in my "Sent" folder is plain text with 
> 7bit encoding, it seems that something in between has re-encoded it (or 
> that thunderbird is lying to me).
> 
> As this is a repeating theme here, it might be a good idea to add an 
> option to submit patches in a less fragile way than inline in email. But 
> then, maybe that's just my particularly annoying combination of 
> thunderbird client and outlook server that keeps messing things up.

It might just be git am. It does display fine in my mua too, it is just
git am doesn't seem to like the raw message:

https://sourceware.org/cgi-bin/get-raw-msg?listname=elfutils-devel&date=2019-q1&msgid=67ff4de2-fd5a-0ba8-0b5d-173c5619a88b%40qt.io

But please do feel free to post patches as pull requests using git
request-pull -p if you have a public git repo (but please use -p and
also sent the output to the list to make reviewing easier).

Cheers,

Mark


Re: [PATCH] Skip run-readelf-compressed.sh test if built without bzip2

2019-01-16 Thread Frank Ch. Eigler
Hi -

> It might just be git am. It does display fine in my mua too, it is just
> git am doesn't seem to like the raw message:
> 
> https://sourceware.org/cgi-bin/get-raw-msg?listname=elfutils-devel&date=2019-q1&msgid=67ff4de2-fd5a-0ba8-0b5d-173c5619a88b%40qt.io

That could be because of sourceware's anti-spammer mangling of the
addresses in email headers.  I don't know if this is worth keeping,
but I use this pipeline for applying get-raw-msg? outputs:

   wget -q -O - "$f" | sed '/^From:/s/ at /@/' | sed '/^From:/s/ dot /./g' | 
git am

- FChE