-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fix libFuzzer minimization command line parsing, should be able to process special chars like ( #3886
Comments
Looks like honggfuzz is creating filenames with special chars, which later confused libFuzzer minimizer. Looking into a fix.
|
It might be worth fixing this bug in libFuzzer: |
@robertswiecki - is it possible to get rid of these special chars/instructions in filenames - "mov____(%rdi),%r12" in "/SIGABRT.PC.7ffff76cef07.STACK.16cd11d9f.CODE.-6.ADDR.0.INSTR.mov____(%rdi),%r12.fuzz" |
@morehouse do you think this should be fixed in libFuzzer? I think |
You can compile it with -D_HF_LINUX_NO_BFD But overall it might be good whatever other component which breaks upon non-standard (yet allowed) filename chars :) |
I'd say fix it in libFuzzer if the fix there is easy enough... A couple ideas that might be easy fixes:
|
It seems when honggfuzz is used testcases aren't minimized at all but in https://oss-fuzz.com/testcase-detail/5757581015646208 (which was found with libFuzzer as far as I can see) the minimized testcase is suspiciously large as well. Its size can be reduced from 384 KB to 131 bytes manually. |
How long did your minimization take. We do minimization 5 rounds, 10 min each. Maybe we need less rounds and more per minimization time. Thoughts ? |
It took about 3 and a half minutes to minimize it from 384 KB to 122 bytes. I ran the fuzzer with
|
@evverx - can you try without -runs=50000, maybe that is the issue, i dont think we add that, and maybe that decreases good repro chances (-runs is like retry :) |
ah wait
runs is just total number of runs, that shouldnt impact this. Let me try redo task just to see if it gets minimized properly. |
@inferno-chromium looks like the task failed:
I was going to restart it but decided not to so as not to mess something up |
basically libfuzzer minimization does not account for crash signature when minimizing. When it minimizes something and we retry and if crash signature does not match, we ignore it. I think it makes sense to ignore smaller testcase if crash stack changes ? Can you try that locally minimized version and see if stack is exactly the same.
|
Interestingly, the backtraces are the same most of the time
|
but occasionally with the same testcase (no matter whether it's minimized or not) I can see
|
Agreed. It seems either the fuzz target can trigger different code paths or MSan somehow reports different issues. I'm not sure it has anything to do with OSS-Fuzz. |
Though I think with flaky fuzz targets like this another option would be to try to minimize testcases for some time and then pick the smallest one triggering the original issue. I'm not sure whether it's worth it though. |
It does that already, it tries 10 different sequential attempts to minimize and saves minimized version if same crash signature occurred or ignore result if it changed. i can completely ignore minimization result for flaky testcases, but i hate wrong results. if we minimized wrong, we cause havoc down the road, like regression testing, etc. |
I agree but this particular fuzz target can crash differently with the same testcase (making it kind of impossible to get the minimization right) so whatever havoc can be caused will most likely be caused as far as I can tell. |
Have a fix at google/clusterfuzz#1804, will get landed next week after review. |
IMO we should always try to use the original testcase for things like regression/progression. Don't know if we end up with some testcases that are too large to use for this purpose though. |
FWIW it appears that fuzz target somehow broke the part of OSS-Fuzz reporting issues (probably as expected). The underlying issue was fixed three days ago and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22736 was verified and closed. 8 minutes later https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22880 was opened and it was open until I manually restarted that task to make sure it was really fixed:
|
It could be that that lag has nothing to do with the flaky backtraces though because another bug (along with three undeduplicated variations) that was fixed yesterday hasn't been closed yet even though according to https://oss-fuzz-build-logs.storage.googleapis.com/index.html#systemd that commit was finally picked up by OSS-Fuzz. |
I don't know that I have the cycles to do this now. Any chance someone from dynamic-tools could fix this? |
@Dor1s - can you please help to fix this in libFuzzer [see https://github.com//issues/3886#issuecomment-637687216] |
So the problem is that honggfuzz creates testcases named e.g. |
yes correct! |
Single quoting works, but not on Windows. A proper solution would be to get rid of I've uploaded https://reviews.llvm.org/D82685, let's see what people think about it. |
seems like @kcc prefers to reject weird file names :) https://reviews.llvm.org/D82685#2133565 |
@inferno-chromium since you assigned me to fix this, our options now are:
|
If we choose 1), we should probably make honggfuzz not to generate such files. Adding extra complexity specific for honggfuzz -> libFuzzer pipeline in CF also doesn't make much sense. |
We can do that filtering when storing testcase, need to filter absolute path and maybe filter bad chars from file basename from path - https://github.com/google/clusterfuzz/blob/master/src/python/datastore/data_handler.py#L721 ? Can also ask Honggfuzz to also not generate such filenames, whichever you prefer. |
I'm fine with sanitizing weird filenames on the CF side then. If libFuzzer isn't going to support weird filenames than we should probably always sanitize file inputs to libFuzzer, but a more local/less thorough fix is fine by me. |
I think sanitization in CF might become a whack-a-mole, as we have multiple sources of testcases, including user provided seed corpora. Since honggfuzz has started causing the problem in the first place, I'd first attempt to convince @robertswiecki to change that behavior or at least provide an option to disable assembly code in the filenames. Robert, would you be willing to make such a change? :) |
To judge from #3886 (comment), it can be compiled with -D_HF_LINUX_NO_BFD |
thanks a lot @evverx, that seems great. FTR, I totally agree that it's better not to use |
I downloaded a reproducer testcase from https://oss-fuzz.com/download?testcase_id=5146387221315584 (related to https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22547) and noticed that it was much larger than it should have been so by manually running the fuzz target with
-minimize_crash=1
I managed to reduce the size from 4333 bytes to just 128 bytes.It seems the testcase wasn't minimized because "LibFuzzer minimization failed" (which is strange in the sense that the bug was found with honggfuzz as far as I can see):
The text was updated successfully, but these errors were encountered: