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

Adding ignore file support #7

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxbeutel
Copy link

Hello!

Wanted to check if it would there would be a possibility to add support for a file where users can specify vulnerability IDs that are supposed to be ignored.

Use case:

  • For example recently we got GO-2023-1621 highlighted "The ScalarMult and ScalarBaseMult methods of the P256 Curve may return an incorrect result if called with some specific unreduced scalars"
  • However, since this only affects very specific use cases, we were quite certain that we won't be affected by this.
  • So instead of upgrading right away, we could've added the vulnerability GO-2023-1621 to some file in the repo like .govulnignore, and then govulncheck would ignore the vulnerability, but still scan for other vulnerabilities.

Aquasec for container scanning supports a .trivyignore file that offers a similar feature (ignoring vulnerabilities), see: https://aquasecurity.github.io/trivy/v0.35/docs/vulnerability/examples/filter/

Implementation

The MR is designed to be a non-breaking change:

  • Add a new command line option -ignore-file which defaults to empty string
  • If the file exists, read it in line by line: Each line is supposed to contain a vulnerability id like GO-2023-1621 which is supposed to be ignored.
  • A lookup set is created from this file
  • The vulnerabilities found by govulncheck are then filtered against this lookup set before getting reported to the user.

Example command line invocation:

$ ../govulncheck/govulncheck -ignore-file=.govulnignore ./...         <<<<< This is the NEW option
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Using go1.20.1 and govulncheck@v0.0.0-5c89caadefca-20230327072231 with
vulnerability data from https://vuln.go.dev (last modified 2023-03-21 23:29:51 +0000 UTC).

Scanning your code and 107 packages across 1 dependent module for known vulnerabilities...

Ignoring vulnerability GO-2023-1621       <<<<< Shows the vuln as ignored
No vulnerabilities found.

$ cat .govulnignore
GO-2023-1621         <<<<< This is how the file looks like, one vuln ID per line

Open tasks

  • Do we want to make the file format more complicated (add comments etc.?)
  • Using a pointer here so that 'ignored' field gets skipped in the output. Is that good?
  • Limitation: Need to resize scanner's capacity for lines over 64K in the ignore file
  • Improve output formatting of ignored vulns: Probably the user should still see more details than just the ID of the vuln?
  • Add more tests

.... And of course in general, if such a feature would be even accepted (which I don't take for granted but I thought it's worth a try.)

Thank you in advance for considering this proposal.

@google-cla
Copy link

google-cla bot commented Mar 27, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Chengxuan
Copy link

Hi @maxbeutel @julieqiu @jba @zpavlinovic , I was thinking of doing the same thing and found this PR. is this something you are considering?

I'm using a workaround script at the moment: https://github.com/kaleido-io/kaleido-sdk-go/blob/7ba2d147a282bd98b707879484667d9a14398cca/govulnchecktool.sh which is prone to changes of json structure of the output data.

@maxbeutel
Copy link
Author

Nice idea @Chengxuan . I was wondering if this PR doesn't get through how to implement ignoring reported vulns. Filtering the output on "client side" is indeed an option, as you demonstrated.

I'm wondering is this the wrong repo to raise the MR? Because I also saw MRs for this project in the main golang repo, under https://github.com/golang/go/labels/vulncheck%20or%20vulndb

@bsiegert
Copy link

@maxbeutel The main Go bug tracker is at https://github.com/golang/go, as you have noticed.

The Go project does not usually use Pull Requests (it uses Gerrit code review), but there is some support for dealing with PRs and converting them to code reviews.

There are a few options for you:

  1. You can continue here, which means signing the CLA and marking the PR as "ready to review".
  2. You can follow https://go.dev/doc/contribute and use Gerrit tooling for creating a code review request.

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