Simple Fuzzing

[kirk-baird]

Updates

I've added a couple of simple fuzz targets using cargo fuzz. They are pretty simple and so should be relatively low maintenance.

Functions fuzzed:

• blake2b()
• blake2bp()
• blake2s()
• blake2sp()

There are also fuzz targets for blake2b and blake2s which test the update() functionality.

Additional Comments

To run these enter the crate then run:

$ cargo fuzz run <fuzzz_target_name>

The fuzz target names can be see as the binaries in fuzz/Cargo.toml or in fuzz/fuzz_targets/fuzz_target_name.rs

E.g.

$ cd blake2b

$ cargo fuzz run fuzz_blake2b

I've kept them pretty simple so maintenance should be minimal. Leaving the fuzzer running overnight should provide sufficient coverage.

I've let them run for about an hour so far an no bugs have been found.

[oconnor663]

Am I right that the fuzzer is looking for panics and unsafe crashes? Would it be possible to have it also look for incorrect outputs? We could compare it to someone else's BLAKE2 implementation, or maybe just to the portable implementation in these crates. There's an undocumented benchmarks::force_portable function that you can use to explicitly run the portable implementation.

[oconnor663]

Another thought: How appropriate would it be to try to integrate something like this into our CI? My guess is "not appropriate" :) But is there a way to explicitly run, say, 1 minute of fuzzing, mainly just to make sure that the fuzzing code stays current?

[kirk-baird]

Am I right that the fuzzer is looking for panics and unsafe crashes? Would it be possible to have it also look for incorrect outputs? We could compare it to someone else's BLAKE2 implementation, or maybe just to the portable implementation in these crates. There's an undocumented benchmarks::force_portable function that you can use to explicitly run the portable implementation.

Currently we are just looking for panics, crashes, overflows etc. What you're talking about is differential fuzzing which takes two implementations and compares the output. Which should be relatively straight forward if there is another rust implementation that we call in a similar manner.

Another thought: How appropriate would it be to try to integrate something like this into our CI? My guess is "not appropriate" :) But is there a way to explicitly run, say, 1 minute of fuzzing, mainly just to make sure that the fuzzing code stays current?

One option is to cd into the fuzz/ directories and call cargo check --all? It'll atleast pick up any compilation issues, which as is should be sufficient.

[oconnor663]

When I run cargo check, I see this error. Am I doing something wrong?

error[E0433]: failed to resolve: use of undeclared type or module `Arbitrary`
 --> fuzz_targets/fuzz_updates.rs:6:10
  |
6 | #[derive(arbitrary::Arbitrary, Debug)]
  |          ^^^^^^^^^^^^^^^^^^^^ use of undeclared type or module `Arbitrary`
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.
error: could not compile `blake2b_simd-fuzz`.

[kirk-baird]

When I run cargo check, I see this error. Am I doing something wrong?

It should run fine when doing cargo check and this branch works on my machine.

So long as the Cargo.toml has

libfuzzer-sys = { version = "0.3", features = ["arbitrary-derive"] }

Then we can include arbitrary in

use libfuzzer_sys::{fuzz_target, arbitrary};

p.s. It's possible that there may be been an update so I'll run rustup update and cargo update and retry when they're done.

[kirk-baird]

I get this issue now I've updated to libfuzzer_sys 0.3.2 which needs arbitrary 0.4.2.

At first glance it may be a bug with the arbitrary derive feature.

[kirk-baird]

I've raise an issue on arbitrary and it's probably better to wait for that to be resolved than implement the workaround as then we'd have to maintain the versions of arbitrary and libfuzzer-sys simultaneously.

[oconnor663]

Thanks for tracking that down. Could you ping this PR when that issue is fixed upstream?

[kirk-baird]

They've released an update in arbitrary 0.4.3 to fix this issue.

Running cargo update in each of the fuzz/ directories should fix the problem :)

[oconnor663]

Hmm, I still see a failure on nightly in CI: https://travis-ci.org/github/oconnor663/blake2_simd/jobs/682431443

=== TEST COMMAND ===
cd /home/travis/build/oconnor663/blake2_simd/blake2b/fuzz && cargo check
    Updating crates.io index
...
    Finished test [unoptimized + debuginfo] target(s) in 25.31s
     Running /home/travis/build/oconnor663/blake2_simd/target/debug/deps/fuzz_blake2b-5f31a88685b1924c
WARNING: Failed to find function "__sanitizer_acquire_crash_state".
WARNING: Failed to find function "__sanitizer_print_stack_trace".
WARNING: Failed to find function "__sanitizer_set_death_callback".
INFO: Seed: 254894105
No such file or directory: check; exiting
error: test failed, to rerun pass '--bin fuzz_blake2b'
The command "(cd run_all_tests && cargo run)" exited with 1.

But it doesn't seem to repro locally. That's weird.

[kirk-baird]

Interesting this looks like rust is having an issue connecting to the LLVM sanitizer.

I think these are found in any of the C++11 or higher compilers.

p.s. I assume this is working on your machine locally?

[oconnor663]

Yep, it does pass locally. Maybe there's some package I could apt-get install on CI?

Incidentally, cargo check also seems to work on stable for me now, at least locally. Is that expected?

[kirk-baird]

You could try apt-get install clang if it is not already installed or gcc.

Otherwise from a CI point of view running cargo check on stable should be sufficient as it will should the code will almost certainly compile on nightly then will run on machines will the correct packages installed.

[oconnor663]

I'm trying various things on this branch: https://github.com/oconnor663/blake2_simd/tree/kirk-baird-fuzz

I might just go ahead and switch to GitHub Actions for CI, since that was likely to happen at some point anyway. If it fixes this build issue, all the better. (Looks like the Travis builds were running on Ubuntu 16.04, which might've been part of the problem.)

Update: Yep, it looks like that fixes the problem.

[oconnor663]

Apologies, I dropped the ball on this one and didn't get around to pushing the merge button until now.