Skip to content

elfutils/readelf#6670

Merged
jonathanmetzman merged 2 commits intogoogle:masterfrom
izzeem:master
Oct 31, 2021
Merged

elfutils/readelf#6670
jonathanmetzman merged 2 commits intogoogle:masterfrom
izzeem:master

Conversation

@izzeem
Copy link
Copy Markdown
Contributor

@izzeem izzeem commented Oct 27, 2021

libelf is used by many distros as the defacto elf parsing library which other applications depend on. elf parsing bugs are usually pretty complex and error prone

i just wrapped readelf as it is a good client of libelf and exercises many of the libraries exported functions

https://packages.debian.org/buster/libelf-dev and https://archlinux.org/packages/core/x86_64/libelf/ as examples

@izzeem izzeem force-pushed the master branch 2 times, most recently from 03fccac to c610506 Compare October 27, 2021 22:14
@izzeem izzeem changed the title elfutils elfutils/readelf Oct 27, 2021
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you the maintainer of this project? Do you have upstream approval for it's inclusion in OSS-Fuzz? Can you ask them if you aren't and don't have approval yet please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sent an email to maintainers to get their approval.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

assistant deputy backup maintainer here, go ahead

Copy link
Copy Markdown
Contributor

@jonathanmetzman jonathanmetzman Oct 28, 2021

Choose a reason for hiding this comment

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

Thanks! Do you want to have access to bugs when they are filed? If so you, please provide an email that @izzeem can add to auto_ccs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jonathanmetzman is there a way to publish artifacts and logs to a mailman mailing list? I think that would be a preferred way for elfutils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No sorry. Each person needs their own google account.
I understand it's inconvenient, but that's all we have support for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@fche libelf is fuzzed indirectly via the libbpf project: https://storage.googleapis.com/oss-fuzz-coverage/libbpf/reports/20211030/linux/src/libbpf/elfutils/report.html and to tell libbpf and libelf bugs apart (or point the build script to commits where bugs have already been fixed) it would be great if I could have access to elfutils bug reports. Would it be OK if I added my email address to auto_cc? Thanks!

@jonathanmetzman jonathanmetzman enabled auto-merge (squash) October 28, 2021 18:55
@jonathanmetzman
Copy link
Copy Markdown
Contributor

Not sure if you fixed build failures. If not, we need to do this before merging.

auto-merge was automatically disabled October 28, 2021 20:06

Head branch was pushed to by a user without write access

@izzeem
Copy link
Copy Markdown
Contributor Author

izzeem commented Oct 28, 2021

Fixed the memory and undefined sanitizer builds, will need an approval on the workflows to see if the other ones passed

@izzeem
Copy link
Copy Markdown
Contributor Author

izzeem commented Oct 29, 2021 via email

@jonathanmetzman jonathanmetzman enabled auto-merge (squash) October 29, 2021 19:47
auto-merge was automatically disabled October 30, 2021 01:10

Head branch was pushed to by a user without write access


# build elf tooling targets
autoreconf -i -f
./configure --enable-maintainer-mode --disable-libdebuginfod --enable-libdebuginfod=dummy --disable-debuginfod --disable-libdebuginfod
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder how it can be built with ASan in its current form? I had to change a couple of files with sed to get libelf to compile with ASan:

# ASan isn't compatible with -Wl,--no-undefined: https://github.com/google/sanitizers/issues/380
find -name Makefile.am | xargs sed -i 's/,--no-undefined//' &&
# ASan isn't compatible with -Wl,-z,defs either:
# https://clang.llvm.org/docs/AddressSanitizer.html#usage
sed -i 's/^\(ZDEFS_LDFLAGS=\).*/\1/' configure.ac &&

Or is https://github.com/izzeem/elfutils somehow different from the main repository hosted at git://sourceware.org/git/elfutils.git?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are the changes I made from the branch to support it. I just keep a separate branch so I can always pull in the upstream.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jonathanmetzman jonathanmetzman enabled auto-merge (squash) October 31, 2021 12:40
@jonathanmetzman
Copy link
Copy Markdown
Contributor

Got rid of i386 for now.
You can fix in another PR (when CI will run automatically for you :-)

@jonathanmetzman jonathanmetzman merged commit 459217e into google:master Oct 31, 2021
evverx added a commit to evverx/oss-fuzz that referenced this pull request Nov 30, 2021
The elfutils project was integrated into OSS-Fuzz in
google#6670 where
Dockerfile pointed to a fork of the official repository
with a series of patches that were supposed to make it compile
on OSS-Fuzz. Apart from that there was a fuzz target that
effectively wrapped the readelf utility by applying a patch
to its source code. On the whole it worked at the time
but I think there are a few issues:

1. It's hard to point OSS-Fuzz to the official repository
(because most of the patches touch the build system and
they can't always be applied cleanly);

2. It's almost impossible to add new fuzz targets covering
other use cases;

3. It's not possible to build fuzz targets without Docker

4. Since the fuzz target mostly wraps the readelf utility
it looks more like a CLI tool than a fuzz target. It calls
exit when it should just return 0 to let it keep going
and so on.

This PR should addresses all those issues apart from 4. The fuzz
target was just removed and another one was added instead. (It can
be added later though but since it isn't exactly maintainable with
the build script pointing at the official repository it should
probably be rewritten:
https://sourceware.org/pipermail/elfutils-devel/2021q4/004295.html)

The new fuzz target covers the code that `systemd` uses to parse
untrusted data. Currently it can be used to trigger various issues
like heap-buffer-overflows and inifinite loops that in theory can bring down
coredump processing on machines where systemd-coredump is used by
default. Even though those issues were discovered by one of `systemd`
fuzz targets I think elfutils bugs should be caught and reported
by elfutils fuzz targets.
@evverx evverx mentioned this pull request Nov 30, 2021
DavidKorczynski pushed a commit that referenced this pull request Dec 1, 2021
The elfutils project was integrated into OSS-Fuzz in
#6670 where
Dockerfile pointed to a fork of the official repository
with a series of patches that were supposed to make it compile
on OSS-Fuzz. Apart from that there was a fuzz target that
effectively wrapped the readelf utility by applying a patch
to its source code. On the whole it worked at the time
but I think there are a few issues:

1. It's hard to point OSS-Fuzz to the official repository
(because most of the patches touch the build system and
they can't always be applied cleanly);

2. It's almost impossible to add new fuzz targets covering
other use cases;

3. It's not possible to build fuzz targets without Docker

4. Since the fuzz target mostly wraps the readelf utility
it looks more like a CLI tool than a fuzz target. It calls
exit when it should just return 0 to let it keep going
and so on.

This PR should addresses all those issues apart from 4. The fuzz
target was just removed and another one was added instead. (It can
be added later though but since it isn't exactly maintainable with
the build script pointing at the official repository it should
probably be rewritten:
https://sourceware.org/pipermail/elfutils-devel/2021q4/004295.html)

The new fuzz target covers the code that `systemd` uses to parse
untrusted data. Currently it can be used to trigger various issues
like heap-buffer-overflows and inifinite loops that in theory can bring down
coredump processing on machines where systemd-coredump is used by
default. Even though those issues were discovered by one of `systemd`
fuzz targets I think elfutils bugs should be caught and reported
by elfutils fuzz targets.
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
The elfutils project was integrated into OSS-Fuzz in
google#6670 where
Dockerfile pointed to a fork of the official repository
with a series of patches that were supposed to make it compile
on OSS-Fuzz. Apart from that there was a fuzz target that
effectively wrapped the readelf utility by applying a patch
to its source code. On the whole it worked at the time
but I think there are a few issues:

1. It's hard to point OSS-Fuzz to the official repository
(because most of the patches touch the build system and
they can't always be applied cleanly);

2. It's almost impossible to add new fuzz targets covering
other use cases;

3. It's not possible to build fuzz targets without Docker

4. Since the fuzz target mostly wraps the readelf utility
it looks more like a CLI tool than a fuzz target. It calls
exit when it should just return 0 to let it keep going
and so on.

This PR should addresses all those issues apart from 4. The fuzz
target was just removed and another one was added instead. (It can
be added later though but since it isn't exactly maintainable with
the build script pointing at the official repository it should
probably be rewritten:
https://sourceware.org/pipermail/elfutils-devel/2021q4/004295.html)

The new fuzz target covers the code that `systemd` uses to parse
untrusted data. Currently it can be used to trigger various issues
like heap-buffer-overflows and inifinite loops that in theory can bring down
coredump processing on machines where systemd-coredump is used by
default. Even though those issues were discovered by one of `systemd`
fuzz targets I think elfutils bugs should be caught and reported
by elfutils fuzz targets.
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.

4 participants