libfuzzer icon indicating copy to clipboard operation
libfuzzer copied to clipboard

Change API to be the same as Honggfuzz-rs and eventually AFL.rs

Open PaulGrandperrin opened this issue 6 years ago • 10 comments

There is a lot of black magic here and this PR is not really intented to be merged directly, principally because it break compatibility.

I understand this code feels very wacky but it leads to a very nice user API:

  • no need to use #![no_main]
  • user can do initialization/setup before the fuzzing starts
  • using macros is not necessary anymore

But really, the very big main advantage is that after this, all fuzzers, libfuzzer-sys, honggfuzz-rs and AFL.rs (after https://github.com/rust-fuzz/afl.rs/pull/137) will be able to share the exact same API!

This will allow to simplify a lot https://github.com/rust-fuzz/targets and easily make cargo-fuzz compatible with all fuzzers.

closes rust-fuzz/cargo-fuzz#119 closes rust-fuzz/cargo-fuzz#101

PaulGrandperrin avatar Apr 28 '18 14:04 PaulGrandperrin

The main drawback I see is that this is using a non-public API of libFuzzer and even worse, a mangled C++ non-public API...

From a pragmatic point of view, it is not a huge risk because any time the libfuzzer code will change, we will have to manually import it and so it'll be easy to do adjustments.

However, I think we should contact the libfuzzer team to ask them to stabilize and export this API in a C format.

PaulGrandperrin avatar Apr 28 '18 15:04 PaulGrandperrin

Very cool!

I don't think this closes rust-fuzz/cargo-fuzz#119 though, see my comment there.

killercup avatar Apr 28 '18 17:04 killercup

Using internals is fine, given that we bundle libfuzzer within this project anyway.

Though, what should be done is adding a separate .cpp file to the vendored libfuzzer copy that exposes a proper extern "C" function to expose the C++ functions, rather than linking to their symbol directly like is done within this PR.

I’m not yet sold on this sort of API sharing, but it is definitely better than nothing. And we can definitely work on making fuzzers swappable through cargo fuzz CLI later on, as @killercup has suggested in the other issue.

nagisa avatar Apr 28 '18 17:04 nagisa

@nagisa yeah reexporting the function via a cpp wrapper is the way to go, I was just too lazy to do it for this proof of concept.

I think the main takeaway from this PR and the other PR on AFL.rs is that we can pretty much abstract whatever API we want to the user, without technical limitations.

Now is probably a good time to debate about which API we want to go forward with and implement it in all 3 fuzzers. I think this discussion can be done here: https://github.com/rust-fuzz/rfcs/pull/1

PaulGrandperrin avatar Apr 28 '18 18:04 PaulGrandperrin

This looks pretty nice. One of my plans for libfuzzer was to patch the cpp code so that instead of the weird linkage dance, you just call start_fuzz(settings) from your main, where settings contains function pointers to TestOneInput and friends. This means we can also do things like replacing the output formatter with Debug output, for example (currently this isn't possible because of the way Rust is compiled -- optionally linked symbols don't work)

Manishearth avatar Apr 28 '18 19:04 Manishearth

What's the status with this? Is there any chance of this getting merged?

ghedo avatar Sep 27 '19 16:09 ghedo

cc @fitzgen we should make sure this fits in with our plans as well

Manishearth avatar Dec 28 '19 08:12 Manishearth

cc @fitzgen we should make sure this fits in with our plans as well

Seems fine to me.

fitzgen avatar Dec 29 '19 21:12 fitzgen

Maybe we should try to publish this crate? And pair that with a change so that cargo fuzz pins to a major version.

+1

I was also thinking about this in the context of using the soon-to-be-released breaking update for arbitrary.

fitzgen avatar Dec 30 '19 03:12 fitzgen

There are a couple things that need fixing, and it needs a rebase as well, I'm going to open a new PR.

Manishearth avatar Dec 30 '19 04:12 Manishearth