-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add ClusterFuzzLite integration #122
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: David Korczynski <david@adalogics.com>
Hi @DavidKorczynski. I've been wanting to add fuzzing and didn't even know about ClusterFuzzLite. Thanks! I've done a bit of reading up and fuzzing using ClusterFuzzLite seems like a great idea. I do have some reservations with regards to the fuzzing harness. The input string is required to be null-terminated and padded with All of that being said, this is a great start! Inputting random data will surely catch some bugs. I'm currently working on getting documentation ready for a first release and plan to add more testing afterwards. Adding your work while adding tests is probably a good plan. I'll get to this in a couple of weeks. With documentation done it'll also be easier to explain some of the limitations/requirements of the api. Thanks again for issuing the PR! |
Should the null-termination happen before/after padding? I added changes such that the there is padding and the null-termination happens after the padding, let me know if this loos better! |
Signed-off-by: David Korczynski <david@adalogics.com>
Null-termination should happen before the padding. So, given |
Signed-off-by: David Korczynski <david@adalogics.com>
Got it -- the fuzzer now null-terminates before the padding. |
Thanks @DavidKorczynski! Looks right. I'll give this a try first thing next week. |
@DavidKorczynski, I'm looking into this. I think I want to be able to run this locally too, i.e. compile with |
options.default_ttl = 3600; | ||
options.default_class = 1; | ||
|
||
zone_parse_string(&parser, &options, &buffers, null_terminated, size_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.
Minor remark: size_of_input
here should be size
.
I studied some examples and came up with something that integrates nicely(?) If |
Corpus is nice, but due to code size I'm not sure it's a concern as such in this case. I think increasing the amount of seconds spend fuzzing may have more impact. Looks good with the flags! There may also be a benefit to having an option that purely relies on forwarding the CFLAGS/CXXFLAGS because these are what OSS-Fuzz provides. Feel free to use the fuzzers however you like though -- no need to merge this PR as such, I'm just happy if we can get continuous fuzzing up and running for simdzone :) |
This adds fuzzing by way of ClusterFuzzLite, which is a GitHub action that will perform a short amount of fuzzing for new PRs. The goal is to use fuzzing to catch bugs that may be introduced by new PRs.
I added a fuzzer that targets
simd_parse_string
, and currently set the timeout of CFLite to 180 seconds. CFLite will flag if the fuzzer finds any issues in the code introduced by a PR.If you'd like to test this the way ClusterFuzzLite runs it (by way of OSS-Fuzz) you can use the steps: