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

Integrate with Go vuln db #679

Open
imjasonh opened this issue Apr 9, 2022 · 10 comments
Open

Integrate with Go vuln db #679

imjasonh opened this issue Apr 9, 2022 · 10 comments

Comments

@imjasonh
Copy link
Member

imjasonh commented Apr 9, 2022

golang.org/x/vuln/vulncheck.Binary "detects presence of vulnerable symbols in exe."

Unlike scanned source code, imports and callstacks aren't available, only the presence of a vulnerable symbol.

There's also a standalone CLI tool to vulncheck a binary, govulncheck

However, it's got this warning at this time:

govulncheck is still experimental and neither its output or the vulnerability database should be relied on to be stable or comprehensive.

Similar to how ko deps fetches and extracts the binary from an image, ko vuln (or scan or something) could fetch and extract the binary then run vulncheck on it to find Go vulnerability database entries.

This ko surface could either simply report on detected vulns, or push a scan result to the registry in whatever form that should take, so other consumers can consume it.

@github-actions
Copy link

github-actions bot commented Jul 9, 2022

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@imjasonh
Copy link
Member Author

imjasonh commented Jul 11, 2022

Played around with the govulncheck CLI a bit. It can operate on source trees or built binaries.

For example, running it on v0.25.0 of Tekton's main packages:

pipeline (v0.25.0)$ govulncheck ./cmd/...
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Scanning for dependencies with known vulnerabilities...
Found 1 known vulnerability.
-------------------------------------------------------

GO-2021-0064
Authorization tokens may be inappropriately logged if the verbosity
level is set to a debug level.

Call stacks in your code:
 github.com/tektoncd/pipeline/cmd/pullrequest-init.main calls github.com/tektoncd/pipeline/pkg/pullrequest.Handler.Download, which eventually calls k8s.io/client-go/transport.debuggingRoundTripper.RoundTrip

Found in:  k8s.io/client-go/[email protected]
Fixed in:  k8s.io/client-go/[email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0064

This took 43 seconds to complete, for all main packages. Just ./cmd/controller took 12s.

This is pretty cool, since it not only shows you what vulns you have, but how you (could) end up hitting it. This would help a lot if the vuln only shows up in cases where some environment variable is set, and you know it's never set that way, or you can at least clearly document not to set it to remain safe.

But, running it on the built binary:

$ crane export gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/controller:v0.25.0 - | tar -Oxf - /ko-app/controller > controller
$ govulncheck controller
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Scanning for dependencies with known vulnerabilities...

And it's been running for 13 minutes with no output 🐌

@imjasonh
Copy link
Member Author

After ~21 minutes:

Found 7 known vulnerabilities.
-------------------------------------------------------

GO-2021-0319
Some big.Int values that are not valid field elements (negative or overflowing)
might cause Curve.IsOnCurve to incorrectly return true. Operating on those values
may cause a panic or an invalid curve operation. Note that Unmarshal will never
return such values.


Found in:  crypto/elliptic@
Fixed in:  crypto/[email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0319


GO-2021-0243
crypto/tls clients can panic when provided a certificate of the
wrong type for the negotiated parameters. net/http clients
performing HTTPS requests are also affected.


Found in:  crypto/tls@
Fixed in:  crypto/[email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0243


GO-2022-0434
Verifying certificate chains containing certificates which are not compliant
with RFC 5280 causes Certificate.Verify to panic on macOS.

These chains can be delivered through TLS and can cause a crypto/tls or
net/http client to crash.


Found in:  crypto/x509@
Fixed in:  crypto/[email protected]
More info: https://pkg.go.dev/vuln/GO-2022-0434


GO-2022-0433
encoding/pem in Go before 1.17.9 and 1.18.x before 1.18.1 has a Decode stack overflow via a large amount of PEM data.


Found in:  encoding/pem@
Fixed in:  encoding/[email protected]
More info: https://pkg.go.dev/vuln/GO-2022-0433


GO-2021-0317
Rat.SetString had an overflow issue that can lead to uncontrolled memory consumption.


Found in:  math/big@
Fixed in:  math/[email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0317


GO-2021-0242
Rat.SetString and Rat.UnmarshalText may cause a panic or an
unrecoverable fatal error if passed inputs with very large
exponents.


Found in:  math/big@
Fixed in:  math/[email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0242


GO-2021-0239
The LookupCNAME, LookupSRV, LookupMX, LookupNS, and LookupAddr
functions and their respective methods on the Resolver type may
return arbitrary values retrieved from DNS which do not follow
the established RFC 1035 rules for domain names. If these names
are used without further sanitization, for instance unsafely
included in HTML, they may allow for injection of unexpected
content. Note that LookupTXT may still return arbitrary values
that could require sanitization before further use.


Found in:  net@
Fixed in:  [email protected]
More info: https://pkg.go.dev/vuln/GO-2021-0239

This seems like less useful output, since it includes more false positives, and no callstack explaining how we use it. Weirdly, the output from scanning the binary doesn't include the vuln reported when scanning the source... 🤔

@imjasonh
Copy link
Member Author

imjasonh commented Sep 6, 2022

new blog post https://go.dev/blog/vuln

@imjasonh
Copy link
Member Author

imjasonh commented Sep 7, 2022

govulncheck seems to have gotten a lot faster. Running against Tekton:

$ time govulncheck ./...
govulncheck ./...  28.74s user 12.87s system 155% cpu 26.789 total

$ go build ./cmd/controller
$ time govulncheck controller
govulncheck controller  2.82s user 0.10s system 98% cpu 2.960 total

So that's nice.

Integrating it in ko probably looks like running govulncheck on the importpath being built (govulncheck ./cmd/controller) while building, since this will give us call stack information that won't be present if we only scan the built binary. Then we can generate and attach a signed attestation for the findings.

Attaching signed attestations probably looks like most of the work necessary for #357 which we want to do anyway.

@developer-guy
Copy link
Collaborator

integrating it in ko probably looks like running govulncheck on the importpath being built (govulncheck ./cmd/controller) while building

It means that ko will require golvuncheck to be installed in the environment where we execute builds with ko, doesn't it?

@imjasonh
Copy link
Member Author

integrating it in ko probably looks like running govulncheck on the importpath being built (govulncheck ./cmd/controller) while building

It means that ko will require golvuncheck to be installed in the environment where we execute builds with ko, doesn't it?

Possibly. Or, if govulncheck functionality is available as a library, we can call it directly. Possibly by then there will be an official first-class go vuln command included in go itself.

@developer-guy
Copy link
Collaborator

developer-guy commented Sep 12, 2022

To directly integrate vulnerability checking into other tools and processes, the vulncheck package exports govulncheck’s functionality as a Go API.

^ https://go.dev/blog/vuln#vulnerability-detection-using-govulncheck

@developer-guy
Copy link
Collaborator

kindly ping @imjasonh, let's implement this 👋

@imjasonh
Copy link
Member Author

I'd love to! I think we'd want to have govulncheck's findings end up in an attestation attached to the built image, and likely signed. This would make it blocked on existing signing work that's been long-awaited (#357)

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

No branches or pull requests

2 participants