-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
infra: add support for non-persistent mode AFL #7343
base: master
Are you sure you want to change the base?
infra: add support for non-persistent mode AFL #7343
Conversation
Also adds an example of using it.
ffba437
to
1ee8bf1
Compare
@vanhauser-thc I would like to add some fuzzers that uses non-persistent afl++ fuzzing, and by the looks of it this could be achieved without major changes. I added a simple example from binutils (but have some other examples to include down the line). Could you give this a review? |
@@ -132,6 +132,10 @@ if [[ "$FUZZING_ENGINE" = afl ]]; then | |||
|
|||
CMD_LINE="$OUT/afl-fuzz $AFL_FUZZER_ARGS -i $CORPUS_DIR -o $FUZZER_OUT $(get_dictionary) $* -- $OUT/$FUZZER" | |||
|
|||
if [[ "$FUZZER" == *"non_persistent"* ]]; then |
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.
Maybe put a TODO here to reference an issue for a long term solution to identifying non-persistent AFL fuzz targets.
@vanhauser-thc Do you have an ideas about how we can identify if a fuzz target uses persistent mode or not?
We can't just look for __afl_manual_init
can we?
Note that LLVMFuzzerTestOneInput will also be in the binary so that the rest of the infra can identify it as a fuzz target.
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, can you change this to TODO(https://github.com/google/oss-fuzz/issues/7347):...
?
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.
the AFL way is to check for the string ##SIG_AFL_PERSISTENT##
in the binary. if that is present, then it is persistent.
for deferred forkserver it is ##SIG_AFL_DEFER_FORKSRV##
note that you can have the forkserver (well you should have one, better performance) without persistent mode - and vice versa.
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.
note that you can have the forkserver (well you should have one, better performance) without persistent mode - and vice versa.
Interesting -- is this by setting N here to 1 https://github.com/AFLplusplus/AFLplusplus/blob/1d4f1e48797c064ee71441ba555b29fc3f467983/utils/aflpp_driver/aflpp_driver.c#L336 using this logic https://github.com/AFLplusplus/AFLplusplus/blob/1d4f1e48797c064ee71441ba555b29fc3f467983/utils/aflpp_driver/aflpp_driver.c#L306 ?
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.
no.
if you use the aflpp driver you automatically have persistent mode. (and a deferred forkserver).
for non-persistent mode you dont need to (and you should not) use aflppdriver
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.
Hey David can you implement a solution based on Marc's feedback.
Basically check if the binary doesn't contain ##SIG_AFL_PERSISTENT##
and then treat it as non-persistent.
Related: #7347 |
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.
lgtm except for my main gripe
@@ -132,6 +132,10 @@ if [[ "$FUZZING_ENGINE" = afl ]]; then | |||
|
|||
CMD_LINE="$OUT/afl-fuzz $AFL_FUZZER_ARGS -i $CORPUS_DIR -o $FUZZER_OUT $(get_dictionary) $* -- $OUT/$FUZZER" | |||
|
|||
if [[ "$FUZZER" == *"non_persistent"* ]]; then |
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.
Hey David can you implement a solution based on Marc's feedback.
Basically check if the binary doesn't contain ##SIG_AFL_PERSISTENT##
and then treat it as non-persistent.
will fix this up shortly. Do you prefer for me to remake this as a PR on a non-fork? |
# TODO: come up with a better solution for indicating when a | ||
# fuzzer is not in persistent mode. | ||
if [[ "$FUZZER" == *"non_persistent"* ]]; then | ||
CMD_LINE="$CMD_LINE @@" |
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.
are all non-persistent targets called like this: ./target inputfile
? without any other command line options? If not maybe this needs a `target.nonpersistent" config file that defines how the target is to be called.
and note that afl++ can also send on stdin
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 exactly, I'm going to make an API for doing this in the .options file we already support.
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.
Why don't we always add the @@ here? That way we don't need to scan the binary, oss fuzz doesn't have to care
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.
a) because otherwise it is not needed, b) I am not sure if the current commit on oss-fuzz of afl++ is one that works fine, or is an older one that has that strong performance decrease otherwise. I want to push an update, but not before next week.
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.
Understood.
I'm just saying, not having to grep through a potentially Chrome-sized binary vs adding an needless commandline flag may be worth it
Of course only makes sense if the target still works then (latest AFL++ version). Maybe keep it in mind :)
Could you clarify this, not sure what it means? |
Ref: #7351 (comment) If I should make this as a branch on github.com/google/oss-fuzz instead of github.com/DavidKorczynski/oss-fuzz so you can run trial builds? |
Sorry for the late reply. |
Fixed |
It would be great if it was possible to resurrect this PR. The systemd project added a test based on a corpus produced by AFL++ in systemd/systemd#27458 the other day. It kind of works but the problem is that the binary has to be built and run manually elsewhere to be able to keep the corpus up to date. It would be easier if it was possible to offload that onto OSS-Fuzz. |
Before I forget apart from |
Also adds an example of using it.