Skip to content
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 2 fuzzers #10475

Closed
wants to merge 4 commits into from
Closed

Add 2 fuzzers #10475

wants to merge 4 commits into from

Conversation

AdamKorcz
Copy link

@AdamKorcz AdamKorcz commented Dec 1, 2020

This PR adds two fuzzers for vault.ParseACLPolicy and random.ParsePolicy respectively.

A bit of context: I have previously worked on fuzzing Vault and the ParseACLPolicy fuzzer has previously found a bug that has led to this fix.

I am therefore committing these two fuzzers with the suggestion of setting up continuous fuzzing for Vault through OSS-fuzz. I will shortly be setting of a draft integration on the OSS-fuzz side, and in case there is interest in completing that integration, all that is needed is to merge the fuzzers and provide at least one maintainers email address. This will allow Google to run the fuzzers continuously and notify maintainers in case bugs are found. The service is offered free of charge to open source projects with the implied expectation that bugs are fixed so that the resources spent on running Vaults fuzzers are put to good use.

Fuzzing has proven effective to find bugs in mature software systems, and in essence it is a technique to test programs whereby pseudo-random data is passed to a target with the goal of uncovering bugs and vulnerabilities. In that regard fuzzing continuously has found vulnerabilities in large projects like Kubernetes in the past.

Naturally a lot more can be done in terms of fuzzing Vault, and these two fuzzers are a way for Vault to get started with fuzz-testing.

The fuzzers in this PR are implemented by way of Dvyukovs go-fuzz engine which is a coverage-guided fuzzer. This means that the fuzzer uses feedback from each run in future input-data creation.

@AdamKorcz
Copy link
Author

AdamKorcz commented Dec 11, 2020

I see that the CircleCI badge also returns "Failed". This does not look related to the files in this commit

Signed-off-by: AdamKorcz <[email protected]>
@AdamKorcz
Copy link
Author

The failing build looks unrelated to the files in this PR. Inputs on this are highly appreciated.

@xntrik
Copy link

xntrik commented Jan 14, 2021

This is looking pretty great @AdamKorcz. I'm wondering that given the fuzzed functions end up being processed inside of https://github.com/hashicorp/hcl whether it would make more sense to add the fuzzers there?

Interestingly, while the current master branch of hcl is v1 (which is used by these Vault methods), the v2 branch currently includes a few fuzz setups already, for example https://github.com/hashicorp/hcl/tree/hcl2/hclsyntax/fuzz.

@AdamKorcz
Copy link
Author

AdamKorcz commented Jan 30, 2021

I'm wondering that given the fuzzed functions end up being processed inside of https://github.com/hashicorp/hcl whether it would make more sense to add the fuzzers there?

To me it is all the same. If you (and other maintainers) prefer to have fuzzers for entrypoints in Vault over at https://github.com/hashicorp/hcl, then sure thing! Should I move them over there?

@xntrik
Copy link

xntrik commented Feb 1, 2021

I think moving them over there makes more sense, as HCLv1 is used in more places than just Vault.

@aphorise
Copy link
Contributor

aphorise commented Aug 18, 2022

Hey @AdamKorcz thank you for all your efforts here. Can you confirm how relevant this may still be for you?

@ncabatoff any chance this can be considered for merging soon?

@biazmoreira
Copy link
Contributor

Hi Adam,

Thanks for the contribution! We think Fuzzing is important, but before introducing it to the codebase or any other related-engine, we think it's best if our product and security teams review it and add examples on how to approach this as well as automating it to be part of our testing suite.

I have opened a request to the Security team and will close this PR for now. If you have an issue associated with this PR, could you please link it here? It will help us keep an eye on requests like these in the future. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants