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

provide for cargo afl, but it is not complete. #352

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

xizheyin
Copy link

@xizheyin xizheyin commented Mar 7, 2024

Hi! I've added initial support for cargo afl, but it's not perfect. I'm not quite sure what the norms are for contributing code, and to what extent it can be merged.

Running cargo llvm-cov afl will run afl for 10 seconds (I'll modify it later to let the user set their own parameters for time, etc.) and get a report.

xizheyin added 4 commits March 7, 2024 23:29
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I would like to use as criteria for merging this whether or not it works in my other projects that use cargo afl.

Currently it uses cargo afl with the following steps:

git clone https://github.com/taiki-e/parse-changelog
cd parse-changelog/fuzz
cargo afl build --release --features afl
cargo afl fuzz -i seeds/parse -o out/parse target/release/parse

If using llvm-cov would it be something like this?

# in fuzz directory
cargo llvm-cov afl --release --features afl -i seeds/parse -o out/parse -V 10 target/release/parse

Or exactly the same as cargo afl?

# in fuzz directory
cargo llvm-cov afl build --release --features afl
cargo llvm-cov afl fuzz -i seeds/parse -o out/parse -V 10 target/release/parse

Comment on lines +536 to +544
// prepare seeds
let seeds = vec!["example input 1", "sample data 2", "test case 3"];

for (index, seed) in seeds.into_iter().enumerate() {
let file_path = format!("{input_dir}seed_{index}.txt");
let path = Path::new(&file_path);
let mut file = fs::File::create(path)?;
file.write_all(seed.as_bytes())?;
}
Copy link
Owner

Choose a reason for hiding this comment

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

I beliave these should not be defined on our end, but should respect what the user passes as arguments.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, I would fix it,

Comment on lines +546 to +553
// set afl's loop count to 1000 to avoid AFL loop infinitely!
// AFL maybe exits a process several minutes later and produce a cov file.
cargo.set("AFL_FUZZER_LOOPCOUNT", "20")?;
cargo.arg("afl").arg("fuzz");
cargo.arg("-V").arg("10");
// set in and out
cargo.arg("-i").arg(input_dir);
cargo.arg("-o").arg(output_dir);
Copy link
Owner

Choose a reason for hiding this comment

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

These probably have the same problem. Even if we define our own defaults, we need to respect them if they are specified by the user. Personally, I would prefer to report an error if an environment variable or argument is not set than to define our own on our end.

Comment on lines +525 to +526
cargo.arg("afl").arg("build");
cargo.arg("--target-dir").arg(cx.ws.target_dir.as_str());
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to propagate flags passed by the user, such as releases. We probably need to create a new function like test_or_run_args in cargo.rs.

@taiki-e
Copy link
Owner

taiki-e commented Mar 9, 2024

cc @smoelius: Do you have any recommendations or opinions on the interface or behavior here? I guess you are the best person to ask about this because you are a cargo-afl maintainer and cargo-llvm-cov contributor:)

@smoelius
Copy link
Contributor

smoelius commented Mar 9, 2024

cc @smoelius: Do you have any recommendations or opinions on the interface or behavior here? I guess you are the best person to ask about this because you are a cargo-afl maintainer and cargo-llvm-cov contributor:)

It's very kind of you to ask. :)

I think your proposed interface makes a lot of sense.

One thing I note about the current PR is that it contains code one would expect to find in cargo-afl, e.g., docs/cargo-llvm-cov-afl.txt.

FWIW, cargo afl subcommand will run cargo subcommand with AFL-instrumented binaries. I'm not sure if this makes your task easier, @xizheyin.

As an aside, someone recently asked for coverage on the afl.rs repo: rust-fuzz/afl.rs#465

@taiki-e

This comment was marked as off-topic.

@xizheyin

This comment was marked as off-topic.

@taiki-e

This comment was marked as off-topic.

@xizheyin
Copy link
Author

@taiki-e
Sorry to interrupt again. If I have 10 different programs that all use a certain library and I have access to their profraws, is it possible to easily count the total coverage of this library by these ten programs?

@taiki-e
Copy link
Owner

taiki-e commented Mar 17, 2024

Sorry to interrupt again. If I have 10 different programs that all use a certain library and I have access to their profraws, is it possible to easily count the total coverage of this library by these ten programs?

If it is possible to (temporarily) combine the crates to be tested into a single workspace to run tests, perhaps it would be easy to do by combining with --dep-coverage.

If not, it would be difficult with the current implementation since only artifacts related to the current workspace would be collected, but I think that could be handled by adjusting object_files (probably just making this if branch unconditional -- of course that is not a reasonable default behavior although), specifying the same target directory by CARGO_TARGET_DIR, and using --no-report + report pattern.

In any case, could you please open a separate issue for questions not directly related to this PR?

@xizheyin
Copy link
Author

Thanks, I will open a issue if I have any question.

@njelich
Copy link

njelich commented Apr 10, 2024

It seems sufficient to just add an "external tests" guide, but for AFL - this seems to work with the tutorial fuzzer in the AFL docs:

source <(cargo llvm-cov show-env --export-prefix)
cargo llvm-cov clean --workspace
cargo afl build
AFL_FUZZER_LOOPCOUNT=20 cargo afl fuzz -c - -V 10 -i in -o out target/debug/url-fuzz-target
cargo llvm-cov report --html

And it allows full customization without overhauling the enitrety of the llvm-cov args system.

@taiki-e taiki-e force-pushed the main branch 2 times, most recently from 932a369 to 67a45bf Compare December 18, 2024 19:57
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.

5 participants