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

Govulncheck : No output of vulnerabilities after Scan #2

Closed
brconnell4 opened this issue Sep 12, 2022 · 20 comments · Fixed by #3
Closed

Govulncheck : No output of vulnerabilities after Scan #2

brconnell4 opened this issue Sep 12, 2022 · 20 comments · Fixed by #3

Comments

@brconnell4
Copy link

brconnell4 commented Sep 12, 2022

I wanted to add Govulncheck to our security scanning and have the vulnerabilities output to the scan results, but they are not showing up as expected. I am running this within Github and came across no errors when CodeQL was running.

When running the scan locally, I get 4 vulnerabilities. I.e

Scanning for dependencies with known vulnerabilities...
Found 4 known vulnerabilities.

Vulnerability #1: GO-2022-0524
  Calling Reader.Read on an archive containing a large number of
  concatenated 0-length compressed files can cause a panic due to
  stack exhaustion.

  Call stacks in your code:
      pkg/keycloak/client.go:263:30: github.com/aaa-ncnu/mem-join-service/pkg/keycloak.Client.VerifyToken calls io/ioutil.ReadAll, which eventually calls compress/gzip.Reader.Read

  Found in: compress/[email protected]
  Fixed in: compress/[email protected]
  More info: https://pkg.go.dev/vuln/GO-2022-0524

But when the scan finishes, it doesn't report on these vulnerabilties.

I have added the following to the bottom of my codeql config

      - name: Golang Vulncheck
        uses: Templum/[email protected]

The task finishes successfully with the following:

Running govulncheck for package ./... in dir /github/workspace
Successfully parsed report
Converted Report to Sarif format
Preparing Report for commit b31c5397a6c5ba55f1c7159679c3cdc130090077 on ref refs/pull/78/merge 
Successfully uploaded Report to Github it received ID 15d3a094-32ca-11ed-80[22](https://github.com/aaa-ncnu/mem-join-service/actions/runs/3039702339/jobs/4894928655#step:7:23)-63891b3c3dea 
Successfully processed uploaded vulncheck report to Github

And the report is blank, I would expect that 4 vulnerabilities show up right?
Screen Shot 2022-09-12 at 11 55 40 AM

Also I checked and verified this is the same output that github's api has as well:

{
  "processing_status": "complete",
  "errors": null,
  "analyses_url": "https://api.github.com/repos/x/y/code-scanning/analyses?sarif_id=15d3a094-32ca-11ed-8022-63891b3c3dea"
}
@Templum
Copy link
Owner

Templum commented Sep 12, 2022

@brconnell4 actually the issue here is that the found vulnerability is within the golang runtime/version which for your project (or locally) appears to be 1.18, the scan is performed with golang 1.19. Therefore the issue is not discovered.

As the action is fresh I did not yet get a chance to look at how to fix/address the issue. As it seems like GitHub does not support arguments being passed to docker based actions

@brconnell4
Copy link
Author

So I figured this out. I cloned your repo down and added my package to it. When I am running the container on 1.19, I get no vulnerabilities. On my local I am running 1.18.3 and get those vulnerabilities above.

In this case I am assuming the expectation is to use this tool reliably, the versions of GO has to match what is in the container and what our package is actually running at.

Correct me if I am wrong, but it sounds like this tool is only useful if we clone a copy of the repo, adjust it to what we expect our code to be running in and then run it in Git.

@Templum
Copy link
Owner

Templum commented Sep 12, 2022

I'm still in the progress of researching GitHub actions as this is my first action. Hopefully, I should be able to come up with a way that allows you to configure a go version.

But it's true that all limitations of govulncheck also apply to this action, e.g. Vulnerabilities are only reported for the go os that is running the scan (Windows vulnerabilities won't be reported for Linux environments).

@brconnell4
Copy link
Author

brconnell4 commented Sep 12, 2022

Hey no worries at all. I think this is a great tool to have and I can see us having value in it already. Just an idea, I am not familiar with Git actions, but what if you had a task prior to the build that just does a replaces on the version using an environment variable? So the user can specific their GO version in their Codeql config and it will build that Go version and run?

If you wanted to avoid a sed, you can also just use a j2 file and set the env variable using that when converting it from actions.yml.j2 to actions.yml

@Templum
Copy link
Owner

Templum commented Sep 12, 2022

Not sure how you are bringing CodeQL into this, it's not related to it. I was under the assumption that users of the action have to configure it/use it like here

@brconnell4
Copy link
Author

Sorry, I am including it with my CodeQL tasks. I am just running it after other tasks. Same way as you linked.

@Templum
Copy link
Owner

Templum commented Sep 12, 2022

I see, was thinking that there is maybe another way of setting it up. So glad to see that it is not the case. Anyways I will see how I can address this issue in the future, ideally, a user will have to specify the version as input.

@Templum
Copy link
Owner

Templum commented Sep 12, 2022

@brconnell4 could you do me a favor and give the following config a test?

      - name: Integration Test
        id: integration-test
        uses: Templum/govulncheck-action@feature/2
        with:
          go-version: 1.18.3

It should install the wished version and run based on it. So you should see the reports. Both name and id are placeholders and can be replaced with whatever you have in place.

@brconnell4
Copy link
Author

brconnell4 commented Sep 13, 2022

So this works locally, but not in Github actions. I see that it builds the container running the version I specify, but it

Run docker run --rm -v $(pwd):/github/workspace --workdir /github/workspace -e GITHUB_TOKEN=*** -e PACKAGE=./... -e VERSION= -e GITHUB_REPOSITORY=service/name -e GITHUB_REF=refs/pull/78/merge -e GITHUB_SHA=242c62b6703a6ebbf064fcc18712e62cbe433bbd templum/govulncheck-action:local
2022/09/13 13:35:52 Env ENCRYPTION_KEY is not set. Using default.
2022/09/13 13:35:52 ENVIRONMENT_NAME: 
2022/09/13 13:35:52 CREATE_USER_ACCOUNTS: 
2022/09/13 13:35:52 ACTIVATE_IDC: 
2022/09/13 13:35:52 Loading json schema: schemas/schemaConfig.json
2022/09/13 13:35:52 Loading json schema: schemas/schemaStartRequest.json
2022/09/13 13:35:52 Loading json schema: schemas/schemaRegisterRequest.json
2022/09/13 13:35:52 Loading json schema: schemas/schemaCreateRequest.json
2022/09/13 13:35:52 Loading json schema: schemas/schemaMembershipTransaction.json
2022/09/13 13:35:52 Loading json schema: schemas/schemaValidateRequest.json
2022/09/13 13:35:52 Loading config: 
2022/09/13 13:35:52 Read failed: open : no such file or directory 
Error: Process completed with exit code 1.

It now appears to be looking at my Dockerfile instead of the one hosted in this repo to do a build. This is only occurring with this branch btw.

@Templum
Copy link
Owner

Templum commented Sep 13, 2022

Thank you for trying out the change and providing me with the feedback. I should be able to work on what you have shared here.

Will ping you again when I have a version to verify.

@Templum
Copy link
Owner

Templum commented Sep 13, 2022

@brconnell4 I pushed a fix for your issue and also let it successfully run in my playground repository. Could you give it another try and let me know if everything works out for you.

@brconnell4
Copy link
Author

Hey there, so it passed and the output is below:

Screen Shot 2022-09-13 at 12 19 01 PM

But, it is missing items that show up in the vuln scan which might be an issue. Here is a full scan output excluding the informational


Vulnerability #1: GO-2022-0524
  Calling Reader.Read on an archive containing a large number of
  concatenated 0-length compressed files can cause a panic due to
  stack exhaustion.

  Call stacks in your code:
      pkg/keycloak/client.go:263:30: github.com/owner/service/pkg/keycloak.Client.VerifyToken calls io/ioutil.ReadAll, which eventually calls compress/gzip.Reader.Read

  Found in: compress/[email protected]
  Fixed in: compress/[email protected]
  More info: https://pkg.go.dev/vuln/GO-2022-0524

Vulnerability #2: GO-2022-0526
  Calling Decoder.Decode on a message which contains deeply nested
  structures can cause a panic due to stack exhaustion.

  Call stacks in your code:
      cmd/server/encode.go:31:19: github.com/owner/service/cmd/server.gobDecodePayload calls encoding/gob.Decoder.Decode

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

Vulnerability #3: GO-2022-0969
  HTTP/2 server connections can hang forever waiting for a clean
  shutdown that was preempted by a fatal error. This condition can
  be exploited by a malicious client to cause a denial of service.

  Call stacks in your code:
      cmd/server/main.go:112:30: github.com/owner/service/cmd/server.main calls net/http.Server.ListenAndServe

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

Vulnerability #4: GO-2022-0525
  The HTTP/1 client accepted some invalid Transfer-Encoding
  headers as indicating a "chunked" encoding. This could
  potentially allow for request smuggling, but only if combined
  with an intermediate server that also improperly failed to
  reject the header as invalid.

  Call stacks in your code:
      cmd/server/main.go:112:30: github.com/owner/service/cmd/server.main calls net/http.Server.ListenAndServe

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

Vulnerability #5: GO-2022-0520
  Client IP adresses may be unintentionally exposed via
  X-Forwarded-For headers. When httputil.ReverseProxy.ServeHTTP is
  called with a Request.Header map containing a nil value for the
  X-Forwarded-For header, ReverseProxy sets the client IP as the
  value of the X-Forwarded-For header, contrary to its
  documentation. In the more usual case where a Director function
  sets the X-Forwarded-For header value to nil, ReverseProxy
  leaves the header unmodified as expected.

  Call stacks in your code:
      pkg/keycloak/client.go:252:31: github.com/owner/service/pkg/keycloak.Client.VerifyToken calls net/http.Client.Do

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

I think it would be nice to include the informational events too in the output, but having the actual vulnerabilities show up in the full report would be great.

@Templum
Copy link
Owner

Templum commented Sep 13, 2022

@brconnell4 could you double check that the tag you used was 1.18.3 and not 1.18? Because to me, it looks like that. This would explain why you only see the vulnerability for GO-2022-0969 as this was not fixed with 1.18.4.

If the above is not true could you provide me with the output when running with -json flag ? So I can see if those vulns where swallowed by the code.

@brconnell4
Copy link
Author

Yea, I double checked that

Run docker build --build-arg GOLANG_VERSION=1.18.3 --build-arg VULNCHECK_VERSION=latest -f $GITHUB_ACTION_PATH/Dockerfile -t templum/govulncheck-action:local  $GITHUB_ACTION_PATH
Sending build context to Docker daemon    258kB

...

 ---> b83cc38124f1
Step 9/13 : FROM golang:$GOLANG_VERSION
1.18.3: Pulling from library/golang
...
Digest: sha256:12d3995156cb0dcdbb9d3edb5827e4e8e1bf5bf92436bfd12d696ec997001a9a
Status: Downloaded newer image for golang:1.18.3
 ---> 65375c930b21
Step 10/13 : ARG VULNCHECK_VERSION=latest
 ---> Running in 2d025e217b5e
Removing intermediate container 2d025e217b5e

I spun up the container as well and get the full output. Where are you wanting the json flag added? Right now they are running on push inside of Github Actions

@Templum
Copy link
Owner

Templum commented Sep 13, 2022

Thanks @brconnell4 for double-checking. Above you shared the output of your local execution.

But, it is missing items that show up in the vuln scan which might be an issue. Here is a full scan output excluding the informational

I'm referring to this, can you run that same scan (locally) and leverage -json. The final command should be looking like this $ govulncheck -json ./...

@brconnell4
Copy link
Author

Ah i see what you mean. Yea see attached.
output.txt

@Templum
Copy link
Owner

Templum commented Sep 14, 2022

@brconnell4 I just pushed my latest local code, it should now detect all of the vulnerabilities. Sadly there are still issues with resolution, meaning that for some calls I actually discover another calling location.

As I don't have the original code it's hard to say if I'm actually finding correct callers and the govulncheck tool is just incorrect.

That aside thank you very much for your feedback and input, really helped me drive the action forward 👍 .

@Templum Templum reopened this Sep 15, 2022
@brconnell4
Copy link
Author

Awesome, Ill see if I can test it today or tomorrow. Thanks, great tool btw and I am sure other people will get great use out of it alongside CodeQL

@Templum
Copy link
Owner

Templum commented Sep 16, 2022

@brconnell4 I was able to work out the resolution issue and created a branch: fix/caller-resolution for it. Feel free to give that a try, it should find all vulnerable code call sites and further give you a small call stack in the message.

image

@Templum
Copy link
Owner

Templum commented Sep 17, 2022

I merged the above-mentioned Branch and will close this Issue. Feel free to create a new Issue if something does not work as expected.

@Templum Templum closed this as completed Sep 17, 2022
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 a pull request may close this issue.

2 participants