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

Improve fuzzer for XCMv4 #2424

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions polkadot/xcm/xcm-simulator/fuzzer/.gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1 @@
hfuzz_target
hfuzz_workspace
cargo
coverage
ccov.zip
output
6 changes: 1 addition & 5 deletions polkadot/xcm/xcm-simulator/fuzzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ publish = false

[dependencies]
codec = { package = "parity-scale-codec", version = "3.6.1" }
honggfuzz = "0.5.55"
ziggy = { version = "0.8", default-features = false }
arbitrary = "1.2.0"
scale-info = { version = "2.10.0", features = ["derive"] }

Expand Down Expand Up @@ -44,7 +44,3 @@ runtime-benchmarks = [
"xcm-builder/runtime-benchmarks",
"xcm-executor/runtime-benchmarks",
]

[[bin]]
path = "src/fuzz.rs"
name = "xcm-fuzzer"
21 changes: 11 additions & 10 deletions polkadot/xcm/xcm-simulator/fuzzer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,36 @@ underflows.
## Install dependencies

```
cargo install honggfuzz
cargo install --force ziggy cargo-afl honggfuzz grcov
```

## Run the fuzzer

In this directory, run this command:

```
cargo hfuzz run xcm-fuzzer
cargo ziggy fuzz
louismerlin marked this conversation as resolved.
Show resolved Hide resolved
```

You can use the following options to improve fuzzing:
- `-j number_of_jobs` for fuzzing using multiple threads
- `-G 1024` to limit the size of inputs to 1024 bytes
- this will improve fuzzing effectiveness, and you can drop the limit once you have good coverage

## Run a single input

In this directory, run this command:

```
cargo hfuzz run-debug xcm-fuzzer hfuzz_workspace/xcm-fuzzer/fuzzer_input_file
cargo ziggy run -i path/to/your/input
```

## Generate coverage

In this directory, run these four commands:
In this directory, run this command:

```
RUSTFLAGS="-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort" \
CARGO_INCREMENTAL=0 SKIP_WASM_BUILD=1 CARGO_HOME=./cargo cargo build
../../../target/debug/xcm-fuzzer hfuzz_workspace/xcm-fuzzer/input/
zip -0 ccov.zip `find ../../../target/ \( -name "*.gc*" -o -name "test-*.gc*" \) -print`
grcov ccov.zip -s ../../../ -t html --llvm --branch --ignore-not-existing -o ./coverage
cargo ziggy cover
```

The code coverage will be in `./coverage/index.html`.
The code coverage will be in `./output/xcm-simulator-fuzzer/coverage/index.html`.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'a> Arbitrary<'a> for XcmMessage {
if let Ok(message) =
DecodeLimit::decode_with_depth_limit(MAX_XCM_DECODE_DEPTH, &mut encoded_message)
{
return Ok(XcmMessage { source, destination, message })
return Ok(XcmMessage { source, destination, message });
}
Err(Error::IncorrectFormat)
}
Expand Down Expand Up @@ -155,6 +155,35 @@ fn run_input(xcm_messages: [XcmMessage; 5]) {
println!();

for xcm_message in xcm_messages {
// We block Transact messages because they can cause panics that are due to the test
// environment, and not XCM
Copy link
Member

Choose a reason for hiding this comment

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

For example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just checked, and got the following:

Unexpected event: RuntimeEvent::MessageQueue(
    Event::ProcessingFailed {
        ...
        error: ProcessMessageError::Unsupported,
    },
)

in this line of the event processing code when processing an invalid Transact call.
@ggwpez @franciscoaguirre do you think this should be handled differently, e.g. by allowing Unsupported messages instead of panicking, and then removing the blocklist?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could allow that. The thing is arbitrary Transacts will call any type of extrinsic, with wrong attributes or anything of the sort. We could record which Transacts give an error and which ones don't and why. In that way we would know much more than just by blocklisting them

fn matches_blocklisted_messages(message: Instruction<()>) -> bool {
matches!(message, Transact { .. })
}
// We check XCM messages recursively for blocklisted messages
fn matches_recursive(message: &mut Instruction<()>) -> Vec<Instruction<()>> {
match message {
DepositReserveAsset { xcm, .. } |
InitiateReserveWithdraw { xcm, .. } |
InitiateTeleport { xcm, .. } |
TransferReserveAsset { xcm, .. } |
SetErrorHandler(xcm) |
SetAppendix(xcm) => Vec::from(xcm.inner()).iter_mut().flat_map(matches_recursive).collect(),
_ => vec![message.clone()],
}
}

if xcm_message
.message
.clone()
.iter_mut()
.flat_map(matches_recursive)
.any(|m| matches_blocklisted_messages(m))
{
println!(" skipping message\n");
continue;
}

if xcm_message.source % 4 == 0 {
// We get the destination for the message
let parachain_id = (xcm_message.destination % 3) + 1;
Expand Down Expand Up @@ -202,36 +231,7 @@ fn run_input(xcm_messages: [XcmMessage; 5]) {
}

fn main() {
#[cfg(fuzzing)]
{
loop {
honggfuzz::fuzz!(|xcm_messages: [XcmMessage; 5]| {
run_input(xcm_messages);
})
}
}
#[cfg(not(fuzzing))]
{
use std::{env, fs, fs::File, io::Read};
let args: Vec<_> = env::args().collect();
let md = fs::metadata(&args[1]).unwrap();
let all_files = match md.is_dir() {
true => fs::read_dir(&args[1])
.unwrap()
.map(|x| x.unwrap().path().to_str().unwrap().to_string())
.collect::<Vec<String>>(),
false => (args[1..]).to_vec(),
};
println!("All_files {:?}", all_files);
for argument in all_files {
println!("Now doing file {:?}", argument);
let mut buffer: Vec<u8> = Vec::new();
let mut f = File::open(argument).unwrap();
f.read_to_end(&mut buffer).unwrap();
let mut unstructured = Unstructured::new(&buffer);
if let Ok(xcm_messages) = unstructured.arbitrary() {
run_input(xcm_messages);
}
}
}
ziggy::fuzz!(|xcm_messages: [XcmMessage; 5]| {
run_input(xcm_messages);
});
}