Fix double crash for solutions with the same filename (#1232) #1236
Conversation
libafl/src/corpus/inmemory_ondisk.rs
Outdated
| @@ -304,7 +305,13 @@ where | |||
| fn save_testcase(&self, testcase: &mut Testcase<I>, idx: CorpusId) -> Result<(), Error> { | |||
| let file_name_orig = testcase.filename_mut().take().unwrap_or_else(|| { | |||
| // TODO walk entry metadata to ask for pieces of filename (e.g. :havoc in AFL) | |||
| testcase.input().as_ref().unwrap().generate_name(idx.0) | |||
| let mut hasher = ahash::RandomState::with_seeds(0, 0, 0, 0).build_hasher(); | |||
There was a problem hiding this comment.
Why always create the hasher, even when it's not used inside the function? Seems wasteful and somewhat pointless?
There was a problem hiding this comment.
generate_name() should use it.
generate_name(idx.0, &mut hasher)
There was a problem hiding this comment.
It doesn't always? Why not create the hasher inside if you need it? Imho an ideal name shouldn't be random, but log the mutations etc.
There was a problem hiding this comment.
Nautilus doesn't use it for example
There was a problem hiding this comment.
The purpose of putting hasher into the argument is to making it clear it is suggested to use hasher to make the testcase names random.
#1232 (comment)
Imho an ideal name shouldn't be random, but log the mutations etc.
Yup, like afl, but not all mutators log the mutation result
There was a problem hiding this comment.
I don't think that's a good solution, just adding a comment should be better. No need to change the API imho.
That being said, we can also consider having one corpus folder for each core somehow
There was a problem hiding this comment.
hmm ok
That being said, we can also consider having one corpus folder for each core somehow
in tlspuffin's case there were 25 testcases with the same name, while there were only 8 cores fuzzing. I don't know what happened, separating folder is fine but it's not enough in this case.
I think the problem is https://github.com/AFLplusplus/LibAFL/blob/main/libafl/src/corpus/inmemory_ondisk.rs#L316
this counter. It's easy to have clashing names if you monotonously increase it.
How do you think about putting the current time in nano sec instead of a simple counter here? then very little chance to have the same name unless the testcases are stored at the very same nano second
There was a problem hiding this comment.
that's something that tlspuffin should do
domenukk
changed the title
tokatoka
force-pushed
the
fix_inmemondisk
branch
from
baeb6f5 to
9096583
Compare
No description provided.