Re: Fuzzing elfutils

Re: Fuzzing elfutils

[Philippe Antoine]

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

Friendly ping on this ?

> Le 22 sept. 2022 à 09:05, Philippe Antoine <p.antoine@catenacyber.fr> a écrit :
> 
> Hello fuzzers,
> 
> I am Philippe Antoine, working on oss-fuzz.
> 
> I implemented a new sanitizer to detect arbitrary file open.
> One of these was discovered in elfutils with target libFuzzer_elfutils_fuzz-dwfl-core
> Cf https://oss-fuzz.com/testcases?open=yes&q=Arbitrary&proj=elfutils
> 
> I would like to know what you think about this. Is this a bug to you ? Or is it expected ?
> Could this be exploited somehow by an attacker to get secrets such as ~/.ssh/id_rsa ?
> 
> Cheers,
> Philippe
> 

Re: Fuzzing elfutils

[Frank Ch. Eigler]

Hi -

> > Cf https://oss-fuzz.com/testcases?open=yes&q=Arbitrary&proj=elfutils

This is inaccessible without logins.

> > I would like to know what you think about this. Is this a bug to
> > you ? Or is it expected ?  [...]

Crashes on crafted inputs are generally bugs.  Security implications
are usually not dire for a tool such as elfutils.


- FChE

Re: Fuzzing elfutils

[Evgeny Vereshchagin]

Hey Philippe,

> I implemented a new sanitizer to detect arbitrary file open.

I think it's an interesting idea. Among other things it seems it can
be used to detect path traversal attacks.
I'm not sure how exactly it works at this point but if apart from
keeping track of the "open" syscall (and its variations)
it could detect attempts to write data to random files it would be great.

> I would like to know what you think about this. Is this a bug to you ? Or is it expected ?
> Could this be exploited somehow by an attacker to get secrets such as ~/.ssh/id_rsa ?

I don't think it can be exploited to expose stuff that shouldn't be exposed.

Thanks,
Evgeny Vereshchagin

Re: Fuzzing elfutils

[Evgeny Vereshchagin]

> > > Cf https://oss-fuzz.com/testcases?open=yes&q=Arbitrary&proj=elfutils
>
> This is inaccessible without logins.

To judge from https://github.com/google/oss-fuzz/tree/master/infra/experimental/SystemSan#arbitrary-file-open
that new experimental fuzzer
isn't documented yet but as far as I can tell it flags "tainted"
strings passed to the open syscall. That backtrace points to
https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdwfl/dwfl_segment_report_module.c;h=28f87f10dd3962082ec4b995f43069ffc4b5e3d4;hb=HEAD#l784
and I think it's a false positive. Looking at
https://github.com/google/oss-fuzz/issues/8497 it seems it should be
possible
to turn it off eventually.

Thanks,
Evgeny Vereshchagin

Re: Fuzzing elfutils

[Philippe Antoine]

Thanks Evgeny.
Could you explain to me why you think it is a false positive ?

> Le 21 oct. 2022 à 21:57, Evgeny Vereshchagin <evverx@gmail.com> a écrit :
> 
>>>> Cf https://oss-fuzz.com/testcases?open=yes&q=Arbitrary&proj=elfutils
>> 
>> This is inaccessible without logins.
> 
> To judge from https://github.com/google/oss-fuzz/tree/master/infra/experimental/SystemSan#arbitrary-file-open
> that new experimental fuzzer
> isn't documented yet but as far as I can tell it flags "tainted"
> strings passed to the open syscall. That backtrace points to
> https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdwfl/dwfl_segment_report_module.c;h=28f87f10dd3962082ec4b995f43069ffc4b5e3d4;hb=HEAD#l784
> and I think it's a false positive. Looking at
> https://github.com/google/oss-fuzz/issues/8497 it seems it should be
> possible
> to turn it off eventually.
> 
> Thanks,
> Evgeny Vereshchagin

Re: Fuzzing elfutils

[Evgeny Vereshchagin]

> Could you explain to me why you think it is a false positive ?

As far as I can tell the idea behind that sanitizer is to mostly flag
suspicious attempts to pass file names that
haven't been sanitized in any away and I agree that in some cases
depending on what happens after those files
are opened it can lead to various issues (and for that reason there
are a lot of static analyzers for example complaining
about "tainted" strings, uncontrolled spheres and stuff like that). In
this particular case files go through __libdw_open_file
and their build ids are checked (which makes it kind of hard to read
passwords, tokens and so on and then expose them).

Thanks,
Evgeny Vereshchagin

Re: Fuzzing elfutils

[Mark Wielaard]

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

Hi Alexander,

On Mon, Dec 29, 2014 at 06:16:45AM +0300, Alexander Cherepanov wrote:
> We start with a bunch of samples.
> 
> The first step is to pick distinct bugs. You do it when you fix issues.
> Let's assume for now that one commit == one bug. Then for every bug...
> 
> It's nice to document who found the bug. This is useful for credit (and
> hence for CVE assignment).
> 
> Even better to document in which bunch of samples (bugzilla attachment or
> something) the bug was found. This useful as a date of disclosure (and hence
> for CVE assignment).
> 
> Even better to document which specific sample triggered the bug. This makes
> it possible to check if the fix is applied correctly. (Perhaps I can do it
> for my samples automatically by retesting them against various commits.)

So for issues found before elfutils 0.161 I think this is all a bit
mixed up. We started out with a bunch of samples from Hano and you
and then when I realized those samples showed both specific bugs and
design issues that made those bugs worse I started fuzzing myself a
bit to find more "patterns". And not all those "samples" were kept.

For elfutils 0.161 there are about 80 patches that fix various issues
found by the fuzzing examples. Some were more "fundemental" than others.
The patches to fix the ELF header and data buffer reading/sanity checking
in libelf, the leb128 parsing issues in libdw and the relocation fixes
in libdwfl solve whole classes of bugs for example. Without those fixes
the other bugs are far worse (because they make reading/writing memory
outside the ELF image far easier).

And I am afraid there are still some fundamental issues left
see https://bugzilla.redhat.com/show_bug.cgi?id=1170810#c7 that really
need to be fixed before we can claim any random sample thrown at
elfutils shouldn't crash, hang or produce bogus results.

The good news is that things seem to have improved a lot and it now
takes hours/days to find new issues instead of minutes. But till we
release 0.162 with at least those 3 fundamental issues fixed I think
keeping strict track of one sample, one bug, one fix is not realistic.

> Then we need to separate security bugs from non-security ones. To do it
> precisely is difficult but it can be done roughly, e.g. overwriting a buffer
> with attacker controlled data is probably a security issue while null
> pointer dereference is probably not (for the usual use-case of elfutils as I
> understand it; if elfutils is used as a library in a bigger program the
> situation becomes more complex).
> Some analysis can be performed automatically based on valgrind/gdb output
> but I'm not sure how good it can be and I haven't yet implemented it. So,
> for now, I'd like to say that it's up to a person who fixes the issue;-)

Since I am the person fixing most issues I am afraid I don't keep track
of whether something is a "security issue" or not. Making elfutils crash,
hang or produce bogus/unreliable results are all pretty bad and just
need to be fixed. Whether they are "exploitable" or not. In my experience
any such issue can be made into a security issue with enough ingenuity.

> Later some folks will want to backport the security fixes to the release of
> elfutils which they have in their distro. For this, they will need to know
> if their version is affected. Your policy in this regard may vary from
> backporting fixes yourself to saying that they just have to migrate to the
> new version. Probably it's enough to check one reasonably old version (I can
> do it for my samples against version 0.152). If you can readily say that,
> e.g., all versions are affected, even better.
> I guess Red Hat / Fedora will also want to backport the fixes? Perhaps they
> have some guidelines for you to follow?

I'll ask. For Fedora I just update the package to the latest upstream
release as soon as possible to fix any bugs found between releases.
 
> Somewhere in the process CVEs can be requested. It seems, now, MITRE assigns
> one CVE to a group of similar issues found by one person, disclosed in one
> day and affecting the same versions. Hence all these pieces of data are
> essential for a CVE request.
> 
> The priorities IMHO should be as follows.
> 
> 1. Pick which commits/fixes are for security issues.
> 
> 2. Everything else (without any order):
> 
> a) match fixes with samples (at least roughly: this patchset fixes issues
> from this bunch of samples; but without mixing issues from different
> researchers);
> 
> b) determining affected versions for each issue.
> 
> It seems I need at least to do some rechecking before requesting CVEs. WDYT,
> is it feasible/reasonable to work this out e.g. for issues from the last
> month?

To be honest, I don't think that is feasible. Just finding, analysing and
fixing the issues found is a lot of work. I try to keep track of what/who
originally triggered some fix. But certainly for fixes before the 0.161
release I don't think splitting them out and catogorizing them separately
is very useful. You will need them all anyway. I think it is reasonable
to just say that we are working towards making it safe to process arbitrary
random ELF files and DWARF debuginfo data with elfutils by 0.162 (to be
released on March). But that in general people should only use elfutils
tools and libraries on files produced by a trusted toolchain for now.

I do realize parsing ELF and DWARF files with elfutils tools will show
up in surprising places and that people cannot always make sure they
files processed are generated by a trusted toolchain and aren't malicious.
Which is why I take your reports very seriously. I am certainly not against
filing CVE requests and trying to figure out exactly what security issue
is fixed by which patch. But realisticly there are just too many issues
at the moment.

Practically I won't be able to help much the next couple of weeks because
I don't have stable access to the internet on vacation. But I'll try and
monitor any new issues found.

Thanks,

Mark

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-05 11:58, Mark Wielaard wrote:
>>> We don't specificly track any security issues, we just treat them as bugs
>>> to be fixed and do a new release when enough/important bugs have been fixed.
>>> There have been people who have filed CVEs against elfutil bugs though.
>>> I don't have any experience with filing CVEs though.
>>
>> I see. For now, I've added 'Security' keyword to the bug in the
>> bugzilla. This should get attention of the security team. Otherwise I
>> can ask for CVEs later in oss-security mailing list.
>
> Thanks, some guidance on how to deal with these issues would be
> appreciated. Sadly at the moment there are just too many to special case
> them. So for now just report issues and we try to fix them asap.

Sorry for the late reply. Unfortunately I don't have complete answers 
but let's try to reason about it. Kinda thinking out loud:-)

We start with a bunch of samples.

The first step is to pick distinct bugs. You do it when you fix issues. 
Let's assume for now that one commit == one bug. Then for every bug...

It's nice to document who found the bug. This is useful for credit (and 
hence for CVE assignment).

Even better to document in which bunch of samples (bugzilla attachment 
or something) the bug was found. This useful as a date of disclosure 
(and hence for CVE assignment).

Even better to document which specific sample triggered the bug. This 
makes it possible to check if the fix is applied correctly. (Perhaps I 
can do it for my samples automatically by retesting them against various 
commits.)

Then we need to separate security bugs from non-security ones. To do it 
precisely is difficult but it can be done roughly, e.g. overwriting a 
buffer with attacker controlled data is probably a security issue while 
null pointer dereference is probably not (for the usual use-case of 
elfutils as I understand it; if elfutils is used as a library in a 
bigger program the situation becomes more complex).
Some analysis can be performed automatically based on valgrind/gdb 
output but I'm not sure how good it can be and I haven't yet implemented 
it. So, for now, I'd like to say that it's up to a person who fixes the 
issue;-)

Later some folks will want to backport the security fixes to the release 
of elfutils which they have in their distro. For this, they will need to 
know if their version is affected. Your policy in this regard may vary 
from backporting fixes yourself to saying that they just have to migrate 
to the new version. Probably it's enough to check one reasonably old 
version (I can do it for my samples against version 0.152). If you can 
readily say that, e.g., all versions are affected, even better.
I guess Red Hat / Fedora will also want to backport the fixes? Perhaps 
they have some guidelines for you to follow?

Somewhere in the process CVEs can be requested. It seems, now, MITRE 
assigns one CVE to a group of similar issues found by one person, 
disclosed in one day and affecting the same versions. Hence all these 
pieces of data are essential for a CVE request.

The priorities IMHO should be as follows.

1. Pick which commits/fixes are for security issues.

2. Everything else (without any order):

a) match fixes with samples (at least roughly: this patchset fixes 
issues from this bunch of samples; but without mixing issues from 
different researchers);

b) determining affected versions for each issue.

It seems I need at least to do some rechecking before requesting CVEs. 
WDYT, is it feasible/reasonable to work this out e.g. for issues from 
the last month?

Please note that what is written here is my personal (and quite rough) 
take on the subject. The usual link for requesting CVEs for Open source 
software is this:

https://people.redhat.com/kseifrie/CVE-OpenSource-Request-HOWTO.html

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Mon, Dec 22, 2014 at 01:20:24AM +0300, Alexander Cherepanov wrote:
> But it would be nice to have an idea which commits fix which crashes. Your
> first commit (d0070a9) included Reported-by: -- thanks! But it was the only
> such commit, then it was not clear whether you fix further crashes from the
> same bunch or fix completely different crashes.

Sorry about that. Once I started to look seriously into the fuzzer crashers
I realized there were some fundemental issue that needed fixed. So I took
your and Hano and my own crashers and just put them into a big directory
running under valgrind looking for common patterns to fix.

> I guess I can formulate two wishes now:
> - to include Reported-by: tag in your commits (to make it possible to track
> progress and as a credit);
> - to let me know when it's reasonable to start next round of fuzzing.

I'll do that. mjw/pending contains 2 patches that should fix your
most recent reported crashers. It is reasonable to start the next round
based on those now.

Thanks,

Mark

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-19 03:13, Mark Wielaard wrote:
> On Thu, 2014-12-18 at 21:15 +0300, Alexander Cherepanov wrote:
>>> Thanks. I'll try to reproduce them soon. But without a general leb128
>>> length check fix using eu-readelf -w might be somewhat unreliable (and
>>> this also might impact -e/--exceptions).
>>
>> There are many patches flowing and it's not clear which are relevant for
>> my crashes and when it's the right time to start fuzzing again.
>
> Now would be a good time :)

Ok:-)

> I am not aware of any pending crashers.
> Although I am aware of 3 areas that still need some work because they
> could potentially cause crashes on bad input (I'll update the bug soon).

It seems the crashes I uploaded yesterday hit at least one of these 
areas (Dwarf_Abbrev) but maybe not others. OTOH you don't need fuzzing 
to hit these areas if you already know they are problematic. Fuzzing is 
more interesting when it uncovers something unexpected.

> Sorry there were so many changes. But sadly there were a lot of
> crashers. I hope we got them all. And some of yours needed some more
> general fixes that needed some discussion. But those patches are now
> finally all in.

The amount of fixes is not something to be sorry about, it's something 
to be proud of:-)

But it would be nice to have an idea which commits fix which crashes. 
Your first commit (d0070a9) included Reported-by: -- thanks! But it was 
the only such commit, then it was not clear whether you fix further 
crashes from the same bunch or fix completely different crashes.

I guess I can formulate two wishes now:
- to include Reported-by: tag in your commits (to make it possible to 
track progress and as a credit);
- to let me know when it's reasonable to start next round of fuzzing.

[skip]
>> Further fuzzing found 3 crashes in readelf. Not sure if you want to look
>> into them now.
>
> Yes, please do add them to the "fuzzer crash bug":
> https://bugzilla.redhat.com/show_bug.cgi?id=1170810

Done.

> Sorry if they don't get immediately fixed for 0.161. But the release
> should already have happened and I like to include just some testsuite
> cleanups and get it shipped, before continuing with more work.

Sure, trying to fix everything will postpone the release indefinitely:-(

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Thu, 2014-12-18 at 21:15 +0300, Alexander Cherepanov wrote:
> > Thanks. I'll try to reproduce them soon. But without a general leb128
> > length check fix using eu-readelf -w might be somewhat unreliable (and
> > this also might impact -e/--exceptions).
> 
> There are many patches flowing and it's not clear which are relevant for 
> my crashes and when it's the right time to start fuzzing again.

Now would be a good time :) I am not aware of any pending crashers.
Although I am aware of 3 areas that still need some work because they
could potentially cause crashes on bad input (I'll update the bug soon).
Sorry there were so many changes. But sadly there were a lot of
crashers. I hope we got them all. And some of yours needed some more
general fixes that needed some discussion. But those patches are now
finally all in.

> Well, I current master against samples which I submitted earlier and it 
> seems everything is fixed except for a couple of invalid reads when 
> processing 6f100f93:
> 
> ==5634== Invalid read of size 1
> ==5634==    at 0x4E43A08: __libdw_get_uleb128 (memory-access.h:65)
> ==5634==    by 0x4E43A08: dwarf_getabbrevattr (dwarf_getabbrevattr.c:63)
> ==5634==    by 0x4097CE: print_debug_abbrev_section (readelf.c:4573)

Yes, that is one area that still needs some improvement. When processing
a "raw" abbrev with dwarf_getabbrevattr we don't know where it
originally came from and so cannot do bounds checks yet. Luckily in this
case it doesn't crash, but I think it potentially could. So we'll need
to add some tracking there.

> Further fuzzing found 3 crashes in readelf. Not sure if you want to look 
> into them now.

Yes, please do add them to the "fuzzer crash bug":
https://bugzilla.redhat.com/show_bug.cgi?id=1170810

Sorry if they don't get immediately fixed for 0.161. But the release
should already have happened and I like to include just some testsuite
cleanups and get it shipped, before continuing with more work.

Thanks,

Mark

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-08 11:52, Mark Wielaard wrote:
> On Mon, 2014-12-08 at 06:06 +0300, Alexander Cherepanov wrote:
>> On 2014-12-05 11:58, Mark Wielaard wrote:
>
>>> Yes, that is true. I have been using afl. And it is good to throw some
>>> other fuzzers at it. The reason you are so successful is because till
>>> now we concentrated on readelf and libelf. Clearly the other tools need
>>> fuzzing too. And we do know debuginfo (-w), libdw, has some known
>>> issues. One of which I just fixed in response to your testcases (see the
>>> patch posted, I haven't pushed it yet, to see if there are any
>>> comments).
>>
>> Ok, I've switched to mjw/pending branch. I hope it's the right branch to
>> have all your latest fixes?
>
> Yes. All patches on there have also been posted to the mailinglist for
> discussion before applying to master. Note that the branch often gets
> rebased once patches are merged (or rewritten) in master. So don't be
> surprised if you get conflicts just git pulling. Best to delete your
> local branch and fetch a new one periodically.

I see, thanks for the info.

>>> I hope to get to the other main libdw debug issue (leb128
>>> parsing) soon. After that hopefully you will have a bit more of a
>>> challenge :)
>>
>> Well, I've uploaded some more crashes for the current (i.e. mjw/pending)
>> readelf. Some of them could be duplicates of the previous unfixed ones.
>
> Thanks. I'll try to reproduce them soon. But without a general leb128
> length check fix using eu-readelf -w might be somewhat unreliable (and
> this also might impact -e/--exceptions).

There are many patches flowing and it's not clear which are relevant for 
my crashes and when it's the right time to start fuzzing again.

Well, I current master against samples which I submitted earlier and it 
seems everything is fixed except for a couple of invalid reads when 
processing 6f100f93:

==5634== Invalid read of size 1
==5634==    at 0x4E43A08: __libdw_get_uleb128 (memory-access.h:65)
==5634==    by 0x4E43A08: dwarf_getabbrevattr (dwarf_getabbrevattr.c:63)
==5634==    by 0x4097CE: print_debug_abbrev_section (readelf.c:4573)
==5634==    by 0x410EF3: print_debug (readelf.c:8247)
==5634==    by 0x410EF3: process_elf_file (readelf.c:897)
==5634==    by 0x410EF3: process_dwflmod (readelf.c:691)
==5634==    by 0x4E55A70: dwfl_getmodules (dwfl_getmodules.c:82)
==5634==    by 0x407C11: process_file (readelf.c:790)
==5634==    by 0x403C33: main (readelf.c:296)
==5634==  Address 0x5e93e5e is 0 bytes after a block of size 318 alloc'd
==5634==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==5634==    by 0x5084A48: convert_data (elf_getdata.c:140)
==5634==    by 0x5084A48: __elf_getdata_rdlock (elf_getdata.c:442)
==5634==    by 0x4E3F7C5: check_section (dwarf_begin_elf.c:130)
==5634==    by 0x4E3FA8A: global_read (dwarf_begin_elf.c:270)
==5634==    by 0x4E3FA8A: dwarf_begin_elf (dwarf_begin_elf.c:378)
==5634==    by 0x4E56B92: load_dw (dwfl_module_getdwarf.c:1213)
==5634==    by 0x4E56D40: find_dw (dwfl_module_getdwarf.c:1240)
==5634==    by 0x4E56D40: dwfl_module_getdwarf (dwfl_module_getdwarf.c:1295)
==5634==    by 0x410DA1: print_debug (readelf.c:8167)
==5634==    by 0x410DA1: process_elf_file (readelf.c:897)
==5634==    by 0x410DA1: process_dwflmod (readelf.c:691)
==5634==    by 0x4E55A70: dwfl_getmodules (dwfl_getmodules.c:82)
==5634==    by 0x407C11: process_file (readelf.c:790)
==5634==    by 0x403C33: main (readelf.c:296)

(that's the first one and the second one differs only in the very first 
address).

Further fuzzing found 3 crashes in readelf. Not sure if you want to look 
into them now.

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Mon, 2014-12-08 at 10:14 +0100, Mark Wielaard wrote:
> On Mon, 2014-12-08 at 04:01 +0300, Alexander Cherepanov wrote:
> > On 2014-12-04 19:03, Mark Wielaard wrote:
> > > Good news, the asserts from readelf-asserts.tar.gz don't trigger anymore
> > > and the command seems to run fine.
> > 
> > Sorry, I'm still seeing "readelf: readelf.c:7751: 
> > print_debug_exception_table: Assertion `readp == action_table' failed." 
> > on 437b7cf1.
> 
> Right, sorry, I didn't include -e in my tests.
> The assert is "right" the Action Table should immediately follow the
> Call site table. But we should just report invalid data if that isn't
> the case instead of asserting.
> 
> Patch attached (and on the mjw/pending branch).

Patch pushed to master now.

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Mon, 2014-12-08 at 04:01 +0300, Alexander Cherepanov wrote:
> On 2014-12-04 19:03, Mark Wielaard wrote:
> > Good news, the asserts from readelf-asserts.tar.gz don't trigger anymore
> > and the command seems to run fine.
> 
> Sorry, I'm still seeing "readelf: readelf.c:7751: 
> print_debug_exception_table: Assertion `readp == action_table' failed." 
> on 437b7cf1.

Right, sorry, I didn't include -e in my tests.
The assert is "right" the Action Table should immediately follow the
Call site table. But we should just report invalid data if that isn't
the case instead of asserting.

Patch attached (and on the mjw/pending branch).

Thanks,

Mark

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-readelf-Report-invalid-data-if-action-table-doesn-t-.patch --]
[-- Type: text/x-patch, Size: 1363 bytes --]

From acfa45c417f7c4feb34d91d8156fd394bf0a5df8 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Mon, 8 Dec 2014 10:11:43 +0100
Subject: [PATCH] readelf: Report invalid data if action table doesn't follow
 call site table.

Don't assert.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 src/ChangeLog | 5 +++++
 src/readelf.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 1c3cf2f..4771dc9 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2014-12-08  Mark Wielaard  <mjw@redhat.com>
+
+	* readelf.c (print_debug_exception_table): Report invalid data if
+	action table doesn't immediately follow call site table.
+
 2014-12-07  Mark Wielaard  <mjw@redhat.com>
 
 	* readelf.c (print_debug_line_section): max_ops_per_instr cannot
diff --git a/src/readelf.c b/src/readelf.c
index fa9ede1..1db54c6 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7748,7 +7748,8 @@ print_debug_exception_table (Dwfl_Module *dwflmod __attribute__ ((unused)),
 		       "        Action:            %u\n"),
 	      u++, call_site_start, call_site_length, landing_pad, action);
     }
-  assert (readp == action_table);
+  if (readp != action_table)
+    goto invalid_data;
 
   unsigned int max_ar_filter = 0;
   if (max_action > 0)
-- 
1.8.3.1

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Mon, 2014-12-08 at 06:06 +0300, Alexander Cherepanov wrote:
> On 2014-12-05 11:58, Mark Wielaard wrote:

> > Yes, that is true. I have been using afl. And it is good to throw some
> > other fuzzers at it. The reason you are so successful is because till
> > now we concentrated on readelf and libelf. Clearly the other tools need
> > fuzzing too. And we do know debuginfo (-w), libdw, has some known
> > issues. One of which I just fixed in response to your testcases (see the
> > patch posted, I haven't pushed it yet, to see if there are any
> > comments).
> 
> Ok, I've switched to mjw/pending branch. I hope it's the right branch to 
> have all your latest fixes?

Yes. All patches on there have also been posted to the mailinglist for
discussion before applying to master. Note that the branch often gets
rebased once patches are merged (or rewritten) in master. So don't be
surprised if you get conflicts just git pulling. Best to delete your
local branch and fetch a new one periodically.

> > I hope to get to the other main libdw debug issue (leb128
> > parsing) soon. After that hopefully you will have a bit more of a
> > challenge :)
> 
> Well, I've uploaded some more crashes for the current (i.e. mjw/pending) 
> readelf. Some of them could be duplicates of the previous unfixed ones.

Thanks. I'll try to reproduce them soon. But without a general leb128
length check fix using eu-readelf -w might be somewhat unreliable (and
this also might impact -e/--exceptions).

Cheers,

Mark

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-05 11:58, Mark Wielaard wrote:
> On Fri, 2014-12-05 at 02:10 +0300, Alexander Cherepanov wrote:
>> On 2014-12-04 17:27, Mark Wielaard wrote:
>>> But I found that using such broad coverage makes the search space for the
>>> fuzzer really, really big. It can take days for the fuzzer to generate a
>>> somewhat valid data for some of the section types. It is imho better to
>>> not use -a or -w, or a combination of flags for different headers or data
>>> sections, but to create a minimal valid ELF file with just one kind of
>>> section or segment and then let the fuzzer run on that with just one
>>> specific flag (or --debug-dump=xxx).
>>
>> I think this is specific to AFL which you seem to use. For it, I agree
>> with your approach. But I'm not sure how useful such an advanced fuzzer
>> at this stage. I'm still using zzuf. Right now it gives more crashes
>> than I can pipe through valgrind. You can get similar behavior with AFL
>> if you specify -dn options (or perhaps you can use just -d).
>
> Yes, that is true. I have been using afl. And it is good to throw some
> other fuzzers at it. The reason you are so successful is because till
> now we concentrated on readelf and libelf. Clearly the other tools need
> fuzzing too. And we do know debuginfo (-w), libdw, has some known
> issues. One of which I just fixed in response to your testcases (see the
> patch posted, I haven't pushed it yet, to see if there are any
> comments).

Ok, I've switched to mjw/pending branch. I hope it's the right branch to 
have all your latest fixes?

> I hope to get to the other main libdw debug issue (leb128
> parsing) soon. After that hopefully you will have a bit more of a
> challenge :)

Well, I've uploaded some more crashes for the current (i.e. mjw/pending) 
readelf. Some of them could be duplicates of the previous unfixed ones.

>>> We don't specificly track any security issues, we just treat them as bugs
>>> to be fixed and do a new release when enough/important bugs have been fixed.
>>> There have been people who have filed CVEs against elfutil bugs though.
>>> I don't have any experience with filing CVEs though.
>>
>> I see. For now, I've added 'Security' keyword to the bug in the
>> bugzilla. This should get attention of the security team. Otherwise I
>> can ask for CVEs later in oss-security mailing list.
>
> Thanks, some guidance on how to deal with these issues would be
> appreciated. Sadly at the moment there are just too many to special case
> them. So for now just report issues and we try to fix them asap.
>
> The short term plan is to do a new release (elfutils 0.161) after the
> next weekend. Get as many bugs fixed before Friday 12th, do some
> additional testing during the weekend and have a release the 15th. This
> is not a very special release, just our periodic ~3/4 month release
> cycle. It will have a lot of robustness fixes though. But I doubt we
> will have fixed all crashers found by then. We will try though.

I'll reply to this a bit later.

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-04 19:03, Mark Wielaard wrote:
> On Thu, 2014-12-04 at 15:27 +0100, Mark Wielaard wrote:
>> Thanks! We have been fixing various issues the last couple of weeks
>> and I just pushed some my fixes to git master. So if you could retry
>> against the very latest git checkout that would be very helpful.
>> I'll run your crashers locally against my tree and will report which
>> issues still exist.
>
> Good news, the asserts from readelf-asserts.tar.gz don't trigger anymore
> and the command seems to run fine.

Sorry, I'm still seeing "readelf: readelf.c:7751: 
print_debug_exception_table: Assertion `readp == action_table' failed." 
on 437b7cf1.

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Fri, 2014-12-05 at 02:10 +0300, Alexander Cherepanov wrote:
> On 2014-12-04 17:27, Mark Wielaard wrote:
> > But I found that using such broad coverage makes the search space for the
> > fuzzer really, really big. It can take days for the fuzzer to generate a
> > somewhat valid data for some of the section types. It is imho better to
> > not use -a or -w, or a combination of flags for different headers or data
> > sections, but to create a minimal valid ELF file with just one kind of
> > section or segment and then let the fuzzer run on that with just one
> > specific flag (or --debug-dump=xxx).
> 
> I think this is specific to AFL which you seem to use. For it, I agree 
> with your approach. But I'm not sure how useful such an advanced fuzzer 
> at this stage. I'm still using zzuf. Right now it gives more crashes 
> than I can pipe through valgrind. You can get similar behavior with AFL 
> if you specify -dn options (or perhaps you can use just -d).

Yes, that is true. I have been using afl. And it is good to throw some
other fuzzers at it. The reason you are so successful is because till
now we concentrated on readelf and libelf. Clearly the other tools need
fuzzing too. And we do know debuginfo (-w), libdw, has some known
issues. One of which I just fixed in response to your testcases (see the
patch posted, I haven't pushed it yet, to see if there are any
comments). I hope to get to the other main libdw debug issue (leb128
parsing) soon. After that hopefully you will have a bit more of a
challenge :)

> > We don't specificly track any security issues, we just treat them as bugs
> > to be fixed and do a new release when enough/important bugs have been fixed.
> > There have been people who have filed CVEs against elfutil bugs though.
> > I don't have any experience with filing CVEs though.
> 
> I see. For now, I've added 'Security' keyword to the bug in the 
> bugzilla. This should get attention of the security team. Otherwise I 
> can ask for CVEs later in oss-security mailing list.

Thanks, some guidance on how to deal with these issues would be
appreciated. Sadly at the moment there are just too many to special case
them. So for now just report issues and we try to fix them asap.

The short term plan is to do a new release (elfutils 0.161) after the
next weekend. Get as many bugs fixed before Friday 12th, do some
additional testing during the weekend and have a release the 15th. This
is not a very special release, just our periodic ~3/4 month release
cycle. It will have a lot of robustness fixes though. But I doubt we
will have fixed all crashers found by then. We will try though.

> [skip]
> > I might be good to have a central place to store these results.
> > The mailinglist seems a little problematic and we might miss/overlook
> > some issues just posted to the list.
> 
> Sure, and mailing several megs of attachments to all the list is not 
> nice too. It's just there is no info about bug reporting on the project 
> page at https://fedorahosted.org/elfutils/ .

Oops. Fixed!

> > Do you have some location where you
> > can store them and any future files? Or could you open a bugzilla report
> > against elfutils and attach them there?
> > https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora
> 
> I think the bugzilla is exactly fine for this. Filed here:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1170810

Thanks!

Mark

Re: Fuzzing elfutils

[Alexander Cherepanov]

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

On 2014-12-04 17:27, Mark Wielaard wrote:
[skip]
> BTW. It is helpful to know which architecture you are running on. Some
> issues only show on a 32bit architecture trying to parse a 64bit ELF file,
> or on little/big endian systems parsing a different endian ELF file.

Yes, sorry, I'm using amd64 now. Maybe will switch to x86 later.

>> I'm not very familiar with elfutils. Which commands give the most code
>> coverage (and shortest run time)? I've used two commands so far:
>>
>>     objdump -rs
>>     readelf -aAdehIlnrsSVcp -w
>>
>> and crashes seem to differ for them.
>
> That does give a biggest coverage. For eu-readelf -w you might want to use
> -N, --numeric-addresses Do not find symbol names for addresses in DWARF data
> which can increase the runtime a lot (we really need to not do a linear
> search to lookup the addresses...).

Thanks for the tip!

> But I found that using such broad coverage makes the search space for the
> fuzzer really, really big. It can take days for the fuzzer to generate a
> somewhat valid data for some of the section types. It is imho better to
> not use -a or -w, or a combination of flags for different headers or data
> sections, but to create a minimal valid ELF file with just one kind of
> section or segment and then let the fuzzer run on that with just one
> specific flag (or --debug-dump=xxx).

I think this is specific to AFL which you seem to use. For it, I agree 
with your approach. But I'm not sure how useful such an advanced fuzzer 
at this stage. I'm still using zzuf. Right now it gives more crashes 
than I can pipe through valgrind. You can get similar behavior with AFL 
if you specify -dn options (or perhaps you can use just -d).

>> BTW does indended use of elfutils include the use against untrusted
>> files and do you track corresponding security issues?
>
> I guess it isn't specifically intended for use against completely untrusted
> files. But it happens of course. Also some of the elfutils libraries (libelf,
> libdw) are used by tools that might process untrusted data. For example
> systemd might use libdw to extract backtraces from core files - which should
> normally be "trusted" because generated by the kernel, but there might be
> bugs in the generation or they might refer to ELF or debug files that a
> hostile user might have prepared. So we are actively working to make it
> work robustly against anything thrown at it.

Nice to hear it!

> We don't specificly track any security issues, we just treat them as bugs
> to be fixed and do a new release when enough/important bugs have been fixed.
> There have been people who have filed CVEs against elfutil bugs though.
> I don't have any experience with filing CVEs though.

I see. For now, I've added 'Security' keyword to the bug in the 
bugzilla. This should get attention of the security team. Otherwise I 
can ask for CVEs later in oss-security mailing list.

[skip]
> I might be good to have a central place to store these results.
> The mailinglist seems a little problematic and we might miss/overlook
> some issues just posted to the list.

Sure, and mailing several megs of attachments to all the list is not 
nice too. It's just there is no info about bug reporting on the project 
page at https://fedorahosted.org/elfutils/ .

> Do you have some location where you
> can store them and any future files? Or could you open a bugzilla report
> against elfutils and attach them there?
> https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora

I think the bugzilla is exactly fine for this. Filed here:

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

-- 
Alexander Cherepanov

Re: Fuzzing elfutils

[Mark Wielaard]

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

On Thu, 2014-12-04 at 15:27 +0100, Mark Wielaard wrote:
> Thanks! We have been fixing various issues the last couple of weeks
> and I just pushed some my fixes to git master. So if you could retry
> against the very latest git checkout that would be very helpful.
> I'll run your crashers locally against my tree and will report which
> issues still exist.

Good news, the asserts from readelf-asserts.tar.gz don't trigger anymore
and the command seems to run fine.

Mixed news, some of the crashes in readelf-crashes.tar.gz have been
fixed (1e76f17f, 66ad10d4). But a lot still crash. The somewhat good
news is that all of the crashes seem to come from either handling
archives or debuginfo, both of which haven't seen much robustness fixes
yet. And most of the crashes are the same in
__libdw_form_val_compute_len which does a strlen and runs out of the
debug section data. We'll need to pass around the length of the data
section and use strnlen here. But still some more work to do.

Bad news, all of the eu-objdump crashes are still there. The good news
is that all but one (af293379) that deals with ar archives again are
simple to fix by some sanity checks. Patch attached and pushed to
master.

Thanks,

Mark

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-objdump-Add-various-sanity-checks-to-guard-against-c.patch --]
[-- Type: text/x-patch, Size: 2998 bytes --]

From d0070a982cfddbff9c3f744b518b4cde539e5e65 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Thu, 4 Dec 2014 17:01:20 +0100
Subject: [PATCH] objdump: Add various sanity checks to guard against corrupted
 data.

Reported-by: Alexander Cherepanov <cherepan@mccme.ru>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 src/ChangeLog |  7 +++++++
 src/objdump.c | 12 +++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 0819c1e..c149a9c 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2014-12-04  Mark Wielaard  <mjw@redhat.com>
+
+	* objdump.c (show_relocs_x): Make sure destshdr exists.
+	(show_relocs_rel): Don't rely on shdr->sh_entsize, use gelf_fsize.
+	(show_relocs_rela): Likewise.
+	(show_relocs): Make sure destshdr, symshdr and symdata exists.
+
 2014-11-30  Mark Wielaard  <mjw@redhat.com>
 
 	* readelf.c (handle_sysv_hash64): Fix overflow check.
diff --git a/src/objdump.c b/src/objdump.c
index 5376447..87290cc 100644
--- a/src/objdump.c
+++ b/src/objdump.c
@@ -389,7 +389,7 @@ show_relocs_x (Ebl *ebl, GElf_Shdr *shdr, Elf_Data *symdata,
 					   ? xndx : sym->st_shndx),
 			       &destshdr_mem);
 
-      if (shdr == NULL)
+      if (shdr == NULL || destshdr == NULL)
 	printf ("<%s %ld>",
 		gettext ("INVALID SECTION"),
 		(long int) (sym->st_shndx == SHN_XINDEX
@@ -418,7 +418,8 @@ show_relocs_rel (Ebl *ebl, GElf_Shdr *shdr, Elf_Data *data,
 		 Elf_Data *symdata, Elf_Data *xndxdata, size_t symstrndx,
 		 size_t shstrndx)
 {
-  int nentries = shdr->sh_size / shdr->sh_entsize;
+  size_t sh_entsize = gelf_fsize (ebl->elf, ELF_T_REL, 1, EV_CURRENT);
+  int nentries = shdr->sh_size / sh_entsize;
 
   for (int cnt = 0; cnt < nentries; ++cnt)
     {
@@ -438,7 +439,8 @@ show_relocs_rela (Ebl *ebl, GElf_Shdr *shdr, Elf_Data *data,
 		  Elf_Data *symdata, Elf_Data *xndxdata, size_t symstrndx,
 		  size_t shstrndx)
 {
-  int nentries = shdr->sh_size / shdr->sh_entsize;
+  size_t sh_entsize = gelf_fsize (ebl->elf, ELF_T_RELA, 1, EV_CURRENT);
+  int nentries = shdr->sh_size / sh_entsize;
 
   for (int cnt = 0; cnt < nentries; ++cnt)
     {
@@ -506,6 +508,8 @@ show_relocs (Ebl *ebl, const char *fname, uint32_t shstrndx)
 	  GElf_Shdr *destshdr = gelf_getshdr (elf_getscn (ebl->elf,
 							  shdr->sh_info),
 					      &destshdr_mem);
+	  if (unlikely (destshdr == NULL))
+	    continue;
 
 	  printf (gettext ("\nRELOCATION RECORDS FOR [%s]:\n"
 			   "%-*s TYPE                 VALUE\n"),
@@ -522,6 +526,8 @@ show_relocs (Ebl *ebl, const char *fname, uint32_t shstrndx)
 	  GElf_Shdr symshdr_mem;
 	  GElf_Shdr *symshdr = gelf_getshdr (symscn, &symshdr_mem);
 	  Elf_Data *symdata = elf_getdata (symscn, NULL);
+	  if (unlikely (symshdr == NULL || symdata == NULL))
+	    continue;
 
 	  /* Search for the optional extended section index table.  */
 	  Elf_Data *xndxdata = NULL;
-- 
1.8.3.1

Re: Fuzzing elfutils

[Mark Wielaard]

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

Hi Alexander,

On Wed, Dec 03, 2014 at 06:16:29PM +0300, Alexander Cherepanov wrote:
> [Please Cc me, I'm not subscribed.]

BTW. Your message didn't hit my INBOX for some reason.
Even though it does appear in the archives:
https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-December/004346.html
Maybe because of the attachements. Sorry for the late/garbled reply.

> I've been fuzzing binutils for some time now and I've spotted that you 
> are also intersted in it. So I have tried to fuzz elfutils. Results 
> attached. valgrind logs or gdb backtraces are included. Cases are 
> deduplicated based on the full stacktrackes as provided by the 
> corresponding tools.

Thanks! We have been fixing various issues the last couple of weeks
and I just pushed some my fixes to git master. So if you could retry
against the very latest git checkout that would be very helpful.
I'll run your crashers locally against my tree and will report which
issues still exist.

BTW. It is helpful to know which architecture you are running on. Some
issues only show on a 32bit architecture trying to parse a 64bit ELF file,
or on little/big endian systems parsing a different endian ELF file.

> I'm not very familiar with elfutils. Which commands give the most code 
> coverage (and shortest run time)? I've used two commands so far:
> 
>    objdump -rs
>    readelf -aAdehIlnrsSVcp -w
> 
> and crashes seem to differ for them.

That does give a biggest coverage. For eu-readelf -w you might want to use
-N, --numeric-addresses Do not find symbol names for addresses in DWARF data
which can increase the runtime a lot (we really need to not do a linear
search to lookup the addresses...).

But I found that using such broad coverage makes the search space for the
fuzzer really, really big. It can take days for the fuzzer to generate a
somewhat valid data for some of the section types. It is imho better to
not use -a or -w, or a combination of flags for different headers or data
sections, but to create a minimal valid ELF file with just one kind of
section or segment and then let the fuzzer run on that with just one
specific flag (or --debug-dump=xxx).

> BTW does indended use of elfutils include the use against untrusted 
> files and do you track corresponding security issues?

I guess it isn't specifically intended for use against completely untrusted
files. But it happens of course. Also some of the elfutils libraries (libelf,
libdw) are used by tools that might process untrusted data. For example
systemd might use libdw to extract backtraces from core files - which should
normally be "trusted" because generated by the kernel, but there might be
bugs in the generation or they might refer to ELF or debug files that a
hostile user might have prepared. So we are actively working to make it
work robustly against anything thrown at it.

We don't specificly track any security issues, we just treat them as bugs
to be fixed and do a new release when enough/important bugs have been fixed.
There have been people who have filed CVEs against elfutil bugs though.
I don't have any experience with filing CVEs though.

> Any feedback is welcome.
>
> Contents:
> 
> objdump-crashes.tar.gz:
> Files: 11
> Errors:
>       10 Invalid read of size ...
>        7 Process terminating with default action of signal 11 (SIGSEGV)
>        3 Process terminating with default action of signal 8 (SIGFPE)
> 
> readelf-crashes.tar.gz:
> Files: 11
> Errors:
>        1 Argument 'size' of function malloc has a fishy (possibly 
> negative) value: ...
>        1 Invalid free() / delete / delete[] / realloc()
>        9 Invalid read of size ...
>        6 Process terminating with default action of signal 11 (SIGSEGV)
>        1 Process terminating with default action of signal 8 (SIGFPE)
> 
> readelf-asserts.tar.gz:
> Files: 2
> Errors:
>        1 No assertion info in gdb backtrace.
>        1 readelf.c:7731: print_debug_exception_table: Assertion `readp 
> == action_table' failed.

I might be good to have a central place to store these results.
The mailinglist seems a little problematic and we might miss/overlook
some issues just posted to the list. Do you have some location where you
can store them and any future files? Or could you open a bugzilla report
against elfutils and attach them there?
https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora

Thanks,

Mark

Fuzzing elfutils

[Alexander Cherepanov]

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

Hi!

[Please Cc me, I'm not subscribed.]

I've been fuzzing binutils for some time now and I've spotted that you 
are also intersted in it. So I have tried to fuzz elfutils. Results 
attached. valgrind logs or gdb backtraces are included. Cases are 
deduplicated based on the full stacktrackes as provided by the 
corresponding tools.

I'm not very familiar with elfutils. Which commands give the most code 
coverage (and shortest run time)? I've used two commands so far:

   objdump -rs
   readelf -aAdehIlnrsSVcp -w

and crashes seem to differ for them.

BTW does indended use of elfutils include the use against untrusted 
files and do you track corresponding security issues?

Any feedback is welcome.

Contents:

objdump-crashes.tar.gz:
Files: 11
Errors:
      10 Invalid read of size ...
       7 Process terminating with default action of signal 11 (SIGSEGV)
       3 Process terminating with default action of signal 8 (SIGFPE)

readelf-crashes.tar.gz:
Files: 11
Errors:
       1 Argument 'size' of function malloc has a fishy (possibly 
negative) value: ...
       1 Invalid free() / delete / delete[] / realloc()
       9 Invalid read of size ...
       6 Process terminating with default action of signal 11 (SIGSEGV)
       1 Process terminating with default action of signal 8 (SIGFPE)

readelf-asserts.tar.gz:
Files: 2
Errors:
       1 No assertion info in gdb backtrace.
       1 readelf.c:7731: print_debug_exception_table: Assertion `readp 
== action_table' failed.

-- 
Alexander Cherepanov

[-- Attachment #2: objdump-crashes.tar.gz --]
[-- Type: application/x-gzip, Size: 945323 bytes --]

[-- Attachment #3: readelf-crashes.tar.gz --]
[-- Type: application/x-gzip, Size: 74477 bytes --]

[-- Attachment #4: readelf-asserts.tar.gz --]
[-- Type: application/x-gzip, Size: 934473 bytes --]