-
-
Notifications
You must be signed in to change notification settings - Fork 330
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
Introduces disabled testcases for splicing #1932
Conversation
…n/discarding) and execution of input
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.
Looks awesome!
@@ -828,9 +828,10 @@ where | |||
if forced { | |||
let _: CorpusId = fuzzer.add_input(self, executor, manager, input)?; | |||
} else { | |||
let (res, _) = fuzzer.evaluate_input(self, executor, manager, input)?; | |||
let (res, _) = fuzzer.evaluate_input(self, executor, manager, input.clone())?; |
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.
do we need the clone here? It's not a super hot path but still wondering if there's some way, like returning the input again or so
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 nested call-stack makes it difficult to return input without breaking changes to evaluate_input_events
, process_execution
, and evaluate_input_with_observers
etc.
I think the cleanest would be to make must_load_initial_inputs
mandatory for State
and in process_execution have if state is load_inital then fuzzer.add_disabled() else return
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.
@domenukk unfortunately I think we do need a clone there, otherwise we'd be cloning down the line in process_execution
when returning the input. I wasn't able to come up with a clean way to include must_load_initial_inputs
in process_execution
either.
libafl/src/mutators/mutations.rs
Outdated
@@ -1138,7 +1138,7 @@ where | |||
} | |||
|
|||
// We don't want to use the testcase we're already using for splicing | |||
let idx = random_corpus_id!(state.corpus(), state.rand_mut()); | |||
let idx = random_corpus_id!(state.corpus(), state.rand_mut(), false); |
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.
For Crossover/Splicing, I think we also need to include disabled (or maybe it should be configurable)
cc @vanhauser-thc ?
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 even have a random chance to pick a disabled one (so a lower probability to pick them, overall)
Probably needs to be fuzzbench-ed again
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.
cc @tokatoka as well
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've enabled the inclusion of disabled entries in the meantime.
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'll check after i finish my pr
maybe add nth_from_all and get_from_all |
it's better to just add another API instead of adding a boolean flag to the existing methods |
…so get() and nth() do not silently fetch disabled entries. * remove boolean flag from random_corpus_id which allowed inclusion of disabled ids and make it into a new function random_corpus_id_with_disabled * update docs
…a separate function insert_disabled
Ah, my bad, I thought you meant like touch(1). |
Maybe we should rename the method? :D |
I have no clue how - but this somehow breaks D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus>cargo build --profile release
Finished release [optimized + debuginfo] target(s) in 0.29s
D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus>copy .\target\release\frida_gdiplus.exe .
1 file(s) copied.
[cargo-make] INFO - Running Task: harness_windows_cmplog_test
D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus>cd "D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus"
D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus>ml64 cmplog_test.asm /subsystem:windows /link /dll /def:cmplog_test.def /entry:dll_main /out:cmplog.dll
Microsoft (R) Macro Assembler (x64) Version 14.38.33135.0
Copyright (C) Microsoft Corporation. All rights reserved.
Microsoft (R) Incremental Linker Version 14.38.33135.0
Copyright (C) Microsoft Corporation. All rights reserved.
/OUT:cmplog_test.exe
cmplog_test.obj
/dll
/def:cmplog_test.def
/entry:dll_main
/out:cmplog.dll
Creating library cmplog.lib and object cmplog.exp
Assembling: cmplog_test.asm
MASM : warning A4018:invalid command-line option : /subsystem:windows
[cargo-make] INFO - Running Task: test_cmplog
D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus>cd "D:\a\LibAFL\LibAFL\fuzzers\frida_gdiplus"
Testing t1...
The system cannot find the file specified.
SUCCESS: The process "frida_gdiplus.exe" with PID 784 has been terminated.
SUCCESS: The process "frida_gdiplus.exe" with PID 4960 has been terminated.
SUCCESS: The process "frida_gdiplus.exe" with PID 4004 has been terminated.
[cargo-make] ERROR - Error while executing command, exit code: 1337
[cargo-make] WARN - Build Failed. |
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.
@R9295
Can you tell me if you are ready
I would run a fuzzbench run for this before merging as it changes how mutations works
@tokatoka I would say it's ready for fuzzbench |
https://www.fuzzbench.com/reports/experimental/2024-04-05-libafl/index.html |
Any idea what could be wrong with frida here? @s1341 maybe? |
@R9295
|
@tokatoka Thanks for the backtrace. I don't have time today, so I'll have a look tomorrow |
I think the issue is that a function, probably in |
This is awesome, thanks for the hard work! |
* introduce disabled field to Testcase * separate executor's processing of execution (adding to corpus/solution/discarding) and execution of input * introduce add_disabled_input function * enable splicing mutators to fetch disabled inputs * reset modified example * clean up * update docs * update docs for count_with_disabled * fix random_corpus_id for splicing mutator not considering disabled entries * fmt * update docs * clippy * fix corpus_btreemap not working * fix clippy warnings * fix python bindings * typo in count_with_disabled implementations * fix certain splicing mutators not considering disabled inputs * rename count_with_disabled to count_all * introduce count_disabled function * update docs for count_all, count_disabled and count * * introduce get_from_all and nth_from_all for corpus implementations so get() and nth() do not silently fetch disabled entries. * remove boolean flag from random_corpus_id which allowed inclusion of disabled ids and make it into a new function random_corpus_id_with_disabled * update docs * remove boolean is_disabled flag from corpus::insert and make it into a separate function insert_disabled * rename do_insert to _insert * make get_from_all inline for cached and inmemory * add missing functions implementation for PythonCorpus prevent writing feedback when adding disabled testcases * fix nth_from_all overfetching enabled corpus entries * fix clippy & rename execute_with_res to execute_no_process * refactor _insert for corpus_btreemap * make LibfuzzerCorpus and ArtifactCorpus to accomodate disabled entries * fix typo * fix missing docs for map field * fix clippy * test * (hopefully) fix CachedOnDiskCorpus using incorrect corpus when caching testcase * fix typo in inmemory_ondisk leading to fetching disabled entry from enabled corpus --------- Co-authored-by: aarnav <[email protected]> Co-authored-by: Dominik Maier <[email protected]>
@R9295 |
On 47c41c2
|
And this is the "Real" backtrace |
The problem is this. |
implements and closes #1670