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

🐛 enable osv-scanner Go call analysis #3893

Closed

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

bug fix

What is the current behavior?

The Vulnerabilities check deducts for Go vulnerabilities which aren't called.

What is the new behavior (if this is a feature change)?**

osv-scanner Go call analysis is enabled. Uncalled vulnerable functions aren't included in the build, and don't affect the score.

  • Tests for the changes have been added (for bug fixes/features)

I'm not sure there's a great way of testing this via unit tests at least. Perhaps an e2e test?

Which issue(s) this PR fixes

Fixes #3891

Special notes for your reviewer

Testing locally on the original repo seems to have resolved the issue.

$ go run main.go --repo github.com/fxamacker/cbor --checks Vulnerabilities --show-details --format json | jq
{
  "date": "2024-02-20T15:24:30-08:00",
  "repo": {
    "name": "github.com/fxamacker/cbor",
    "commit": "afbafc4054c62138633ff49dfcded790b11e44de"
  },
  "scorecard": {
    "version": "",
    "commit": "unknown"
  },
  "score": 10.0,
  "checks": [
    {
      "details": null,
      "score": 10,
      "reason": "0 existing vulnerabilities detected",
      "name": "Vulnerabilities",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/main/docs/checks.md#vulnerabilities",
        "short": "Determines if the project has open, known unfixed vulnerabilities."
      }
    }
  ],
  "metadata": null
}

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

The Vulnerabilities check now ignores uncalled Go vulnerabilities.

Projects that dont call the vulnerable functions aren't included in the build

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock requested a review from a team as a code owner February 20, 2024 23:26
@spencerschrock spencerschrock requested review from justaugustus and raghavkaul and removed request for a team February 20, 2024 23:26
@spencerschrock
Copy link
Member Author

@hogo6002 would you mind taking a look at this? I couldn't find a way of asking osvscanner.DoScan for all stable call analyses, so for now I just enabled Go explicitly.

Additionally in the commit message I claim "uncalled vulnerable functions aren't included in the build", which I think is true for Go, but figured someone on OSV would know better.

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Merging #3893 (f05e75d) into main (e9af90c) will decrease coverage by 4.59%.
The diff coverage is 42.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3893      +/-   ##
==========================================
- Coverage   75.11%   70.52%   -4.59%     
==========================================
  Files         234      234              
  Lines       15860    15867       +7     
==========================================
- Hits        11913    11190     -723     
- Misses       3187     3967     +780     
+ Partials      760      710      -50     

@spencerschrock
Copy link
Member Author

/scdiff generate Vulnerabilities

Copy link

osv-scanner doesn't throw VulnerabilitiesFoundErr if only uncalled vulnerabilities are found.
For repos that have a mix of called and uncalled, we'll need to check each one.

Signed-off-by: Spencer Schrock <[email protected]>
@hogo6002
Copy link

hogo6002 commented Feb 21, 2024

Hi Spencer,

I couldn't find a way of asking osvscanner.DoScan for all stable call analyses, so for now I just enabled Go explicitly.

Yes, right now, the stable call analysis only contains Go. All stable call analysis states: https://github.com/google/osv-scanner/blob/ef3a2a478d4d7460f80a7fe962b14a3d75a3095f/cmd/osv-scanner/scan/callanalysis_parser.go#L3

Additionally in the commit message I claim "uncalled vulnerable functions aren't included in the build", which I think is true for Go, but figured someone on OSV would know better.

Even with go call analysis on, osvscanner.DoScan still returns both called and uncalled vulns. models.AnalysisInfo {Called: true} models.AnalysisInfo {Called: false}

@spencerschrock
Copy link
Member Author

I'm trying to create a local test. A project which uses an old version of Go intentionally.
Based on my understanding of https://osv.dev/vulnerability/GO-2023-2102:
http.ListenAndServe is vulnerable for versions before 1.20.10 (or 1.21.3)

package main

import (
    "log"
    "net/http"
)

func main() {
    err := http.ListenAndServe(":8080", nil)
    if err != nil {
        log.Fatal(err)
    }
}

the example go.mod declares a vulnerable version:

module example.com/foo/bar

go 1.19

But osv-scanner seems to also care what my Go version is when determining reachability (go.mod still says 1.19).

$ go version
go version go1.22.0 linux/amd64

$ osv-scanner -r .
Scanning dir .
Scanned /tmp/go/go.mod file and found 1 package
Uncalled vulnerabilities 
│ https://osv.dev/GO-2023-2375 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2041 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2043 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2102 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2185 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2186 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2382 │      │ Go        │ stdlib  │ 1.19    │ go.mod │

Vs if I use an older Go toolchain (example go.mod is still 1.19).

$ go version
go version go1.21.2 linux/amd64
$ osv-scanner -r .
Scanning dir .
Scanned /tmp/go/go.mod file and found 1 package
(called vulnerabilities section)
│ https://osv.dev/GO-2023-2102 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2185 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2382 │      │ Go        │ stdlib  │ 1.19    │ go.mod │

Uncalled vulnerabilities
│ https://osv.dev/GO-2023-2375 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2041 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2043 │      │ Go        │ stdlib  │ 1.19    │ go.mod │
│ https://osv.dev/GO-2023-2186 │      │ Go        │ stdlib  │ 1.19    │ go.mod │

Just trying to understand this behavior before enabling it. As our weekly scan runs with the latest version of the Go toolchain

Comment on lines +55 to +57
CallAnalysisStates: map[string]bool{
"go": true,
},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting a temporary merge hold while discussing exactly how this would work

x448 added a commit to x448/cbor that referenced this pull request Feb 21, 2024
Remove OpenSSF Scorecard badge from README.md until
this OpenSSF scorecard bug is fixed:
ossf/scorecard#3891

The fix is in progress at OpenSSF Scorecard:
ossf/scorecard#3893
@hogo6002
Copy link

But osv-scanner seems to also care what my Go version is when determining reachability (go.mod still says 1.19).

@spencerschrock Thanks for reporting this issue. We just merged a fix PR (google/osv-scanner#818) for it.

The installed Go version does not matter to osv-scanner when generating the reachability results.
But be mindful that we might assume a vulnerability has been called if it doesn't have specific function-level details. This information is usually missing from GHSAs unless their aliases have it.

@spencerschrock
Copy link
Member Author

But osv-scanner seems to also care what my Go version is when determining reachability (go.mod still says 1.19).

@spencerschrock Thanks for reporting this issue. We just merged a fix PR (google/osv-scanner#818) for it.

Is osv-scanner planning on a patch release, or should Scorecard consider doing a pseudo-version update to pull the patch in?

The installed Go version does not matter to osv-scanner when generating the reachability results. But be mindful that we might assume a vulnerability has been called if it doesn't have specific function-level details. This information is usually missing from GHSAs unless their aliases have it.

Scorecard currently assumes all reported OSV vulns are called, so I think that would be identical to how things are now.

@hogo6002
Copy link

hogo6002 commented Feb 26, 2024

Is osv-scanner planning on a patch release, or should Scorecard consider doing a pseudo-version update to pull the patch in?

Yes, we will do a release this week.

@spencerschrock
Copy link
Member Author

So this works locally, but when run as a docker image (which is what the cron and scorecard action will do), enabling call analysis doesn't actually do anything because the docker image only has the scorecard binary, not a copy of the Go binary

@spencerschrock spencerschrock deleted the bug/vuln-go-call-analysis branch March 10, 2024 22:19
ssuriyan7 pushed a commit to ssuriyan7/cbor that referenced this pull request Mar 16, 2024
Remove OpenSSF Scorecard badge from README.md until
this OpenSSF scorecard bug is fixed:
ossf/scorecard#3891

The fix is in progress at OpenSSF Scorecard:
ossf/scorecard#3893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

scorecard started reducing score for vulnerabilities in unrelated packages that aren't imported
3 participants