-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
base: main
Are you sure you want to change the base?
Conversation
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>
There was a problem hiding this 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
// 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())?; | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
// 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); |
There was a problem hiding this comment.
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.
cargo.arg("afl").arg("build"); | ||
cargo.arg("--target-dir").arg(cx.ws.target_dir.as_str()); |
There was a problem hiding this comment.
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.
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, As an aside, someone recently asked for coverage on the afl.rs repo: rust-fuzz/afl.rs#465 |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
@taiki-e |
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 In any case, could you please open a separate issue for questions not directly related to this PR? |
Thanks, I will open a issue if I have any question. |
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:
And it allows full customization without overhauling the enitrety of the llvm-cov args system. |
932a369
to
67a45bf
Compare
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.