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 vault fuzzers #458

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add vault fuzzers #458

wants to merge 1 commit into from

Conversation

AdamKorcz
Copy link

This PR adds two fuzzers that were originally submitted directly to Vault: hashicorp/vault#10475. It was suggested on the PR to submit the fuzzers here.

As mentioned on the Vault PR, one of the fuzzers found a bug that was fixed here: #410. The bug was submitted privately.

I am therefore committing these two fuzzers with the suggestion of setting up continuous fuzzing for HCL through OSS-fuzz - This is something I will be happy to do. 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.

I have recently published an article about the importance of running fuzzers continuously.

@apparentlymart
Copy link
Contributor

Hi @AdamKorcz,

While it does seem reasonable to expand the fuzzing surface here, I don't think it's appropriate for HCL to be importing packages from the Vault module to do it.

I see there was already some discussion in hashicorp/vault#10475 about the fact that HCL 2 already has fuzzer implementations (in hclsyntax/fuzz and json/fuzz, but reading between the lines on the other things you referred to here I think perhaps your intent was to add fuzzing to the old HCL 1 implementation, which now lives in the maintenance branch hcl1. (The main branch is now HCL 2.)

With that said, I think it would be fine to add new fuzzer implementations to HCL 1 too, since there are applications like Vault still using it, but we'd need to rework it to talk directly to the HCL API rather than indirectly through the Vault API, because it's not the HCL codebase's responsibility to fuzz test code in the Vault repository. It seems like Vault is calling into hcl.Parse and hcl.DecodeObject, which would both be good candidates for fuzzing here in the HCL repository (albeit in the hcl1 branch) if you're interested.

The Vault team might still be interested in including what you originally contributed as an additional layer of fuzzing that covers the extra steps Vault takes after HCL parsing and decoding, but I'm not on the Vault team so I'll have to leave them to decide how to integrate that.

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 9, 2021

CLA assistant check
All committers have signed the CLA.

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

Successfully merging this pull request may close these issues.

None yet

3 participants