vuln icon indicating copy to clipboard operation
vuln copied to clipboard

Adding ignore file support

Open maxbeutel opened this issue 1 year ago • 8 comments

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 [email protected] 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.

maxbeutel avatar Mar 27 '23 07:03 maxbeutel

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.

google-cla[bot] avatar Mar 27 '23 07:03 google-cla[bot]

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.

Chengxuan avatar Apr 03 '23 15:04 Chengxuan

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

maxbeutel avatar Apr 04 '23 11:04 maxbeutel

@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.

bsiegert avatar May 14 '23 09:05 bsiegert

What's the current status of this?

AkihiroSuda avatar Jun 20 '24 05:06 AkihiroSuda

Note that this is a mirror repo and we don't accept pull requests. This is also something that would need a proposal.

We are not looking to add ignore files at the moment. Note that govulncheck provides json (as well as sarif and openvex) output, so users can write their own simple wrappers that can do the filtering.

zpavlinovic avatar Jun 20 '24 06:06 zpavlinovic

@AkihiroSuda I share @zpavlinovic 's opinion that the feature requested here feels like a level above, it is worth looking at https://github.com/aquasecurity/trivy if you are not aware of it. It covers the scan tool wrap-around logic that is not golang specific.

Chengxuan avatar Jun 20 '24 06:06 Chengxuan

Note that govulncheck provides json (as well as sarif and openvex) output, so users can write their own simple wrappers that can do the filtering.

The lack of the standardization will incur reinventing a bunch of similar but incompatible wrappers.

it is worth looking at https://github.com/aquasecurity/trivy if you are not aware of it

I'm aware of it, and I don't think that having an extra dependency on Trivy should be (practically) necessary for scanning Go programs.

Also, until the upstream govulncheck officially supports ignorelist, program maintainers will continue to receive false-positive reports from people who just run govulncheck.

AkihiroSuda avatar Jun 20 '24 07:06 AkihiroSuda