Add advanced fuzzer #724

maxammann · 2024-04-18T09:34:52Z

Adds a fuzzer which:

Can call contracts
Uses complete scripts with fuzzed script data
Can use LibAFL

Manual

cargo install cargo-fuzz
apt install clang pkg-config libssl-dev # for LibAFL
rustup component add llvm-tools-preview --toolchain nightly

Generate Seeds

It is necessary to first convert Sway programs into a suitable format for use as seed input to the fuzzer. This can be done with the following command:

cargo run --bin seed <input dir> <output dir>

Running the Fuzzer

The Rust nightly version is required for executing cargo-fuzz. We also disable AddressSanitizer for a significant speed improvement, as we do not expect memory issues in a Rust program that does not use a significant amount of unsafe code, which our cargo-geiger analysis showed. It makes sense to leave AddressSanitizer turned on if the Fuel project uses more unsafe Rust in the future (either directly or through dependencies). The remaining flags are either required for LibAFL or are useful to make it use seven cores.

cargo +nightly fuzz run --sanitizer none grammar_aware -- \
	-ignore_crashes=1 -ignore_timeouts=1 -ignore_ooms=1 -fork=7

If you use libfuzzer (default) then the following command is enough:

cargo fuzz run --sanitizer none grammar_aware

Execute a Test Case

Test cases can be executed using the following command. This is useful for triaging issues.

cd fuzz/
cargo run --bin execute <file/dir>

Collect Statistics

We created a tool that writes gas statistics to a file called gas_statistics.csv. This can be used to analyze the execution time versus gas usage on a test corpus.

cargo run --bin collect <file/dir>

Generate Coverage

Regardless of how inputs are generated, it is important to measure a fuzzing campaign’s coverage after its run. To perform this measure, we used the support provided by cargo-fuzz and rustc. First, install cargo-binutils. After that, execute the following command:

cargo +nightly fuzz coverage grammar_aware corpus/grammar_aware

Finally, generate an HTML file using LLVM:

cargo cov -- show
target/x86_64-unknown-linux-gnu/coverage/x86_64-unknown-linux-gnu/release/grammar_aware --format=html -instr-profile=coverage/grammar_aware/coverage.profdata /root/audit/fuel-vm > index.html

The last argument to cargo cov defines the filter based on the host file paths.

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests
If performance characteristic of an instruction change, update gas costs as well or make a follow-up PR for that
The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

TODO

Rust SDK
Sway compiler
Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
Someone else?

CLAassistant · 2024-04-18T09:35:01Z

All committers have signed the CLA.

maxammann · 2024-04-18T09:35:55Z

fuel-asm/src/macros.rs

@@ -1007,6 +1007,7 @@ macro_rules! impl_instructions {
        /// Solely the opcode portion of an instruction represented as a single byte.
        #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
        #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+        #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]


This is required for the previous fuzzing harness, which I fixed.

maxammann · 2024-04-18T09:36:05Z

fuel-crypto/src/secp256/signature_format.rs

@@ -55,6 +55,8 @@ impl TryFrom<secp256k1::ecdsa::RecoveryId> for RecoveryId {
 /// Panics if the highest bit of byte at index 32 is set, as this indicates non-normalized
 /// signature. Panics if the recovery id is in reduced-x form.
 pub fn encode_signature(mut signature: [u8; 64], recovery_id: RecoveryId) -> [u8; 64] {
+    // This assertion is hit during fuzzing. Verify it is safe to disable.
+    #[cfg(not(any(fuzzing, feature = "test-helpers")))]


Worth investigating. Is this assertion expected to be violated during fuzzing?

@Dentosal Why do we have an assertion here? It looks like the user can use an invalid signature, causing the crash. Maybe we need to return an error

maxammann · 2024-04-18T09:36:16Z

fuel-tx/src/transaction/validity.rs

@@ -112,6 +112,8 @@ impl Input {
                    recover_address()?
                };

+                // This error is reached during fuzzing often, verify it is safe to disable it
+                #[cfg(not(any(fuzzing, feature = "test-helpers")))]
                if owner != &recovered_address {


Worth investigating. Is this assertion expected to be violated during fuzzing?

maxammann · 2024-04-18T09:36:41Z

fuel-vm/fuzz/src/lib.rs

+    let max_program_length = 2usize.pow(18)
+        - test_context.get_tx_params().tx_offset()
+        - <fuel_vm::prelude::Script as Script>::script_offset_static()
+        - 256; // TODO reevalute this one


I'm not sure where this magic value comes from, but I guess it is oke to have that.

maxammann · 2024-04-18T09:42:48Z

@xgreenx Let me know how you want the corpus to be delivered :) I suggest an external repository that can be public or private. Private might be better to start with.

I wanted to experiment with integrating a CI fuzzing, but that can follow up as separate PR.

xgreenx

Just an overall question: do you have any ideas on how it can be integrated into our CI on a daily basis?

xgreenx · 2024-04-23T08:46:21Z

fuel-crypto/src/secp256/signature_format.rs

@@ -55,6 +55,8 @@ impl TryFrom<secp256k1::ecdsa::RecoveryId> for RecoveryId {
 /// Panics if the highest bit of byte at index 32 is set, as this indicates non-normalized
 /// signature. Panics if the recovery id is in reduced-x form.
 pub fn encode_signature(mut signature: [u8; 64], recovery_id: RecoveryId) -> [u8; 64] {
+    // This assertion is hit during fuzzing. Verify it is safe to disable.
+    #[cfg(not(any(fuzzing, feature = "test-helpers")))]


@Dentosal Why do we have an assertion here? It looks like the user can use an invalid signature, causing the crash. Maybe we need to return an error

fuel-vm/fuzz/Cargo.toml

xgreenx · 2024-04-24T10:03:24Z

fuel-tx/src/transaction/validity.rs

@@ -112,6 +112,8 @@ impl Input {
                    recover_address()?
                };

+                // This error is reached during fuzzing often, verify it is safe to disable it
+                #[cfg(not(any(fuzzing, feature = "test-helpers")))]


I'm curious what problem you had here and how to reproduce it? Maybe we will have some ideas on how to address that in a better way

I don't have one at hand anymore, but removing the line and running the fuzzer should uncover it quickly. We can simply remove that for now and re-add it later on when you have CI fuzzing.

maxammann · 2024-04-25T09:43:17Z

Just an overall question: do you have any ideas on how it can be integrated into our CI on a daily basis?

My idea was to experiment with https://google.github.io/clusterfuzzlite/ to add CI fuzzing in a future PR.

maxammann · 2024-04-26T09:40:22Z

fuel-tx/src/transaction/validity.rs

+                // This error is reached during fuzzing often, verify it is safe to disable it
+                #[cfg(not(any(fuzzing, feature = "test-helpers")))]


Suggested change

// This error is reached during fuzzing often, verify it is safe to disable it

#[cfg(not(any(fuzzing, feature = "test-helpers")))]

Commit suggestion if you agree with that approach.

maxammann · 2024-04-30T17:18:52Z

@xgreenx Here is the CFL PR: #727

maxammann · 2024-05-13T09:21:39Z

@xgreenx I think we want to add the no-changelog tag to this PR. Also the wasm checks seem to depend on the base branch being in the fuel-vm repository.

xgreenx

I will merge it to our local branch to fix CI and add small modifications=)

* Add initial fuzz files * Fix original fuzz test * Add byte-based script functions back for fuzzing * Disable some errors for fuzzing * Fix new fuzz test due to changes in fuel-vm * Remove unused target * Add readme

* Add advanced fuzzer (#724) * Add initial fuzz files * Fix original fuzz test * Add byte-based script functions back for fuzzing * Disable some errors for fuzzing * Fix new fuzz test due to changes in fuel-vm * Remove unused target * Add readme * feat: Store example corpus in version control * chore: Instructions for how to generate a seed corpus * chore: Remove unused dependencies in binaries * fix: Cargo fmt * fix: Clippy * fix: Pass `fuzzing` as rustc flag when compiling fuzz binaries * feat: Remove broken `grammar_aware` fuzz target in favor of `grammar_aware_advanced` * feat: Default to LibAFL fuzzer * docs: Improve code coverage instructions * feat: Use feature flags to toggle between libFuzzer and LibAFL * docs: Further improvements to code coverage instructions * docs: Clarify what the execute binary does * fix: Minor cleanup * chore: Remove `arbitrary` dependency * feat: Link investigation ticket to ignored assertions * fix: Resurrect proptest arbitrary usage in `fuel-merkle` crate * fix: Remove gas statistics output * fix: typo in fuel-vm/fuzz/README.md Co-authored-by: AurelienFT <[email protected]> * chore: Remove example corpus and update README.md * fix: Minor enhancements from PR review * fix: Remove redundant magic 256 term * fix: Cargo fmt * chore: Bump secp256k1 version and re-enable disabled code paths for fuzzing * chore: Add changelog entry * fix: Clippy fix * fix: Format * Update fuel-crypto/src/secp256/signature_format.rs * chore: Add supported rust version to fuzzer readme * refactor: use `expect` instead of `unwrap` in a lot of places * refactor: Apply review suggestions * fix: More unwrap -> expect --------- Co-authored-by: Max Ammann <[email protected]> Co-authored-by: Mårten Blankfors <[email protected]> Co-authored-by: Mårten Blankfors <[email protected]> Co-authored-by: AurelienFT <[email protected]>

maxammann added 6 commits April 10, 2024 13:45

Add initial fuzz files

ac6f16b

Fix original fuzz test

45aebab

Add byte-based script functions back for fuzzing

316d5f1

Disable some errors for fuzzing

41fcc54

Fix new fuzz test due to changes in fuel-vm

ce87222

Remove unused target

7c5bc7b

maxammann changed the title ~~Tob fuzz~~ Add advanced fuzzer Apr 18, 2024

maxammann commented Apr 18, 2024

View reviewed changes

maxammann force-pushed the tob-fuzz branch from ec73edc to 7c5bc7b Compare April 18, 2024 09:37

maxammann commented Apr 18, 2024

View reviewed changes

maxammann marked this pull request as ready for review April 22, 2024 12:10

xgreenx reviewed Apr 24, 2024

View reviewed changes

Add readme

eaff98c

maxammann commented Apr 26, 2024

View reviewed changes

xgreenx requested a review from Dentosal May 2, 2024 09:18

xgreenx assigned maxammann May 2, 2024

xgreenx changed the base branch from master to feature/tob-fuzzer May 13, 2024 10:23

xgreenx approved these changes May 13, 2024

View reviewed changes

xgreenx merged commit e42702f into FuelLabs:feature/tob-fuzzer May 13, 2024
8 of 11 checks passed

xgreenx mentioned this pull request May 13, 2024

Add advanced fuzzer (#724) #733

Merged

2 tasks

xgreenx mentioned this pull request Sep 17, 2024

Release v0.57.0 #828

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add advanced fuzzer #724

Add advanced fuzzer #724

maxammann commented Apr 18, 2024

CLAassistant commented Apr 18, 2024 •

edited

Loading

maxammann Apr 18, 2024

maxammann Apr 18, 2024

xgreenx Apr 23, 2024

maxammann Apr 18, 2024

maxammann Apr 18, 2024

maxammann commented Apr 18, 2024

xgreenx left a comment

xgreenx Apr 23, 2024

xgreenx Apr 24, 2024

maxammann Apr 26, 2024

maxammann commented Apr 25, 2024

maxammann Apr 26, 2024

maxammann Apr 26, 2024

maxammann commented Apr 30, 2024

maxammann commented May 13, 2024

xgreenx left a comment

		// This error is reached during fuzzing often, verify it is safe to disable it
		#[cfg(not(any(fuzzing, feature = "test-helpers")))]

Add advanced fuzzer #724

Add advanced fuzzer #724

Conversation

maxammann commented Apr 18, 2024

Manual

Generate Seeds

Running the Fuzzer

Execute a Test Case

Collect Statistics

Generate Coverage

Checklist

Before requesting review

After merging, notify other teams

CLAassistant commented Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxammann commented Apr 18, 2024

xgreenx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxammann commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxammann commented Apr 30, 2024

maxammann commented May 13, 2024

xgreenx left a comment

Choose a reason for hiding this comment

CLAassistant commented Apr 18, 2024 •

edited

Loading