Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elfutils: add and expand library fuzzers #7395

Merged
merged 5 commits into from
Mar 16, 2022

Conversation

DavidKorczynski
Copy link
Collaborator

No description provided.

@AdamKorcz AdamKorcz merged commit aa83381 into google:master Mar 16, 2022
@evverx
Copy link
Contributor

evverx commented Mar 21, 2022

It would be great if seed corpora of some kind could be added to let for example ./infra/helper run_fuzzer start triggering relevant codepaths a bit faster.

-I. -I./lib -I./libelf -I./libebl -I./libdw -I./libdwelf -I./libdwfl -I./libasm \
-c "$SRC/fuzz-libdwfl.c" -o fuzz-libdwfl.o
$CXX $CXXFLAGS $LIB_FUZZING_ENGINE fuzz-libdwfl.o \
./libasm/libasm.a ./libebl/libebl.a ./backends/libebl_backends.a ./libcpu/libcpu.a \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why it's linked against these libraries? I opened #7412 where I dropped them but if it's intentional it would be great if there was a comment or something like that. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intentions were to expand with further fuzzers shortly so the idea was to include all the relevant elfutils libraries and then shrink the build set up to a loop instead of many individual calls to $CXX -- in this event it would be easier to link in all static libraries so as to add a new fuzzer you simply add the fuzzer name. Similar to what we did for the net-snmp library https://github.com/net-snmp/net-snmp/blob/8e14effb6626bbf5bc344ec17fdf0354b5b9a90d/testing/fuzzing/build-fuzz-tests.sh#L7-L20

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks! I'll go ahead and close that PR then.

I think as long as issues like #7357 aren't triggered it should be possible to link all the fuzz targets with the common set of the libraries.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, sounds good and thanks for pointing #7357 out.

evverx added a commit to evverx/elfutils that referenced this pull request Mar 21, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Mar 23, 2022
evverx added a commit to evverx/oss-fuzz that referenced this pull request Mar 23, 2022
Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45633.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script
```
When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: [email protected]. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.
```

It's a follow-up to google#7395
and google#7393.
jonathanmetzman pushed a commit that referenced this pull request Mar 24, 2022
Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45633.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script
```
When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: [email protected]. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.
```

It's a follow-up to #7395
and #7393.
evverx added a commit to evverx/elfutils that referenced this pull request Mar 25, 2022
@evverx
Copy link
Contributor

evverx commented Mar 25, 2022

Looks like the fuzzers report the same issues with different backtraces like https://sourceware.org/bugzilla/show_bug.cgi?id=29000. I haven't looked at the code yet but it appears fuzz_logic_once triggers the same code paths as fuzz-libdwfl. I think to avoid duplicates like that (that CF can't deduplicate due to different backtraces) fuzz-libelf should probably be trimmed a bit. I wonder if fuzz-libdwfl already covers the code fuzzed by fuzz_logic_once? If so it can probably be removed.

@DavidKorczynski
Copy link
Collaborator Author

Looks like the fuzzers report the same issues with different backtraces like https://sourceware.org/bugzilla/show_bug.cgi?id=29000. I haven't looked at the code yet but it appears fuzz_logic_once triggers the same code paths as fuzz-libdwfl. I think to avoid duplicates like that (that CF can't deduplicate due to different backtraces) fuzz-libelf should probably be trimmed a bit. I wonder if fuzz-libdwfl already covers the code fuzzed by fuzz_logic_once? If so it can probably be removed.

I'll take a look at this end of next week

evverx added a commit to evverx/elfutils that referenced this pull request Mar 30, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Apr 3, 2022
@DavidKorczynski
Copy link
Collaborator Author

I took a look at things @evverx

There is some intersection between the two in that libdwfl uses libelf to do some of its processing. However, they are different in that libelf uses fuzz-libelf functions directly and fuzz-libdwfl uses libelf by way of libdwfl logic. I would prefer to have more verbose as is and would prefer to keep both fuzzer without trimming.

What is the specific concern though? is it:

  • multiple issues on OSV that are duplicates? Not too sure if I feel that this is something oss-fuzz projects should be too concerned about.
  • double work in reporting to maintainers? Perhaps I could assist in that we could set up some arrangement where I handle the reporting of issues to the mailing list for the fuzzers I wrote
  • avoiding too much CPU power?
  • something else?

@evverx
Copy link
Contributor

evverx commented Apr 4, 2022

What is the specific concern though?

It's the OSV database mostly. I'd agree that maintainers shouldn't be concerned about it if https://osv.dev/ mentioned that in its current form it isn't suitable for processing and reporting "vulnerabilities" automatically (partly due to some OSS-Fuzz issues). But as long as it can still be used to waste maintainers' time I'm not sure it can be ignored completely. It's also being discussed in #7479 (comment).

I would prefer to have more verbose as is and would prefer to keep both fuzzer without trimming.

Let's just keep them as is then. It's not uncommon for fuzz targets to fuzz the same code after all. It's just that usually to avoid duplicates like that it makes sense to add fuzz targets one at a time once "shallow" bugs are fixed.

evverx added a commit to evverx/elfutils that referenced this pull request Apr 5, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Apr 23, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Apr 25, 2022
evverx added a commit to evverx/elfutils that referenced this pull request May 24, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Jun 19, 2022
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
* elfutils: make name more appropriate

* elfutils: update libelf fuzzer

* elfutils: add fuzz-libdwfl

* elfutils: nits

* elfutils: fix build
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
Unlike fuzz-dwfl-core, the new fuzz targets actually use zlib so
instead of just linking against zlib to make them compile they
should use the library instrumented with MSan. Without it OSS-Fuzz
reports bogus issues like https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45630
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45631 and
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=45633.

To hopefully make it easier to figure out how to add new fuzz targets
going forward I also added the following comment to the build script
```
When new fuzz targets are added it usually makes sense to notify the maintainers of
the elfutils project using the mailing list: [email protected]. There
fuzz targets can be reviewed properly (to make sure they don't fail to compile with -Werror
for example), their names can be chosen accordingly (so as not to spam the mailing
list with bogus bug reports that are opened and closed once they are renamed) and so
on. Also since a lot of bug reports coming out of the blue aren't exactly helpful
fuzz targets should probably be added one at a time to make it easier to keep track
of them.
```

It's a follow-up to google#7395
and google#7393.
evverx added a commit to evverx/oss-fuzz that referenced this pull request Sep 18, 2022
Now fuzz-libdwfl and fuzz-libelf can be run a few times in a row
with files triggering crashes.

It's another follow-up to google#7395
and google#7393.
jonathanmetzman pushed a commit that referenced this pull request Sep 18, 2022
Now fuzz-libdwfl and fuzz-libelf can be run a few times in a row
with files triggering crashes.

It's another follow-up to #7395
and #7393.
evverx added a commit to evverx/elfutils that referenced this pull request Nov 27, 2022
evverx added a commit to evverx/elfutils that referenced this pull request Feb 21, 2023
evverx added a commit to evverx/elfutils that referenced this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants