Skip to content

Commit

Permalink
Fix regression for go call analysis in 1.7.0 (google#926)
Browse files Browse the repository at this point in the history
Fixes google#914 

govulncheck behaviour changed either in 1.0.3 or 1.0.4.

govulncheck 1.0.2 used to essentially ignore performing call analysis
for OSV entries without any symbol information.
govulncheck 1.0.4 now attempts to perform call analysis assuming that
everything all symbols are vulnerable, so any calls to any
functions/symbols are considered a vulnerable function called. This is
the case for most GHSA entries.

OSV-Scanner already accounted for OSV entries that doesn't have symbol
level information, and assuming that it affects everything, if there are
no aliased vulnerability entries that does contain symbol information
which is not called.

the govulncheck change resulted in GHSA entries being incorrectly
recorded as having symbol level information, and those symbols have been
called, resulting in the following output:

```
"experimentalAnalysis": {
  "GHSA-c3h9-896r-86jm": {
    "called": true
  },
  "GO-2021-0053": {
    "called": false
  }
}
```

When presented with differing called results, the table output defaults
to it being called.

Our existing tests did not pick this up, because we didn't have an e2e
test, and the unit tests assumed only advisories with symbol information
affected the output.

---

Contents of PR:

- Add extra check when govulncheck returns `called=true` to only pass
the result on when advisory actually contains symbol information.
- Add integration/e2e test for call analysis for go, this way any
behavior changes will be picked up in the future.
- Edited the fixtures slightly for our existing tests to fail when this
new check that's been added is not present.

We could add a GHSA entry without symbol information to the unit tests,
but that results in a snapshot >3000 lines long because govulncheck will
return traces for everything.
  • Loading branch information
another-rex authored Apr 17, 2024
1 parent 8cffd2e commit a40c813
Show file tree
Hide file tree
Showing 9 changed files with 519 additions and 61 deletions.
40 changes: 40 additions & 0 deletions cmd/osv-scanner/__snapshots__/main_test.snap
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,46 @@ No issues found

---

[TestRunCallAnalysis/Run_with_govulncheck - 1]
Scanning dir ./fixtures/call-analysis-go-project
Scanned <rootdir>/fixtures/call-analysis-go-project/go.mod file and found 4 packages
+-------------------------------------+------+-----------+-----------------------------+---------+------------------------------------------+
| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | SOURCE |
+-------------------------------------+------+-----------+-----------------------------+---------+------------------------------------------+
| https://osv.dev/GHSA-2h6c-j3gf-xp9r | 5.9 | Go | github.com/ipfs/go-bitfield | 1.0.0 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-1558 | | | | | |
| https://osv.dev/GO-2023-2375 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-2102 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-2185 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-2382 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2598 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2599 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2687 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
+-------------------------------------+------+-----------+-----------------------------+---------+------------------------------------------+
| Uncalled vulnerabilities | | | | | |
+-------------------------------------+------+-----------+-----------------------------+---------+------------------------------------------+
| https://osv.dev/GHSA-c3h9-896r-86jm | 8.6 | Go | github.com/gogo/protobuf | 1.3.1 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2021-0053 | | | | | |
| https://osv.dev/GHSA-qgc7-mgm3-q253 | 5.5 | Go | golang.org/x/image | 0.4.0 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-1572 | | | | | |
| https://osv.dev/GHSA-j3p8-6mrq-6g7h | 6.5 | Go | golang.org/x/image | 0.4.0 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-1990 | | | | | |
| https://osv.dev/GHSA-x92r-3vfx-4cv3 | 6.5 | Go | golang.org/x/image | 0.4.0 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-1989 | | | | | |
| https://osv.dev/GO-2023-2041 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-2043 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2023-2186 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2600 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2609 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
| https://osv.dev/GO-2024-2610 | | Go | stdlib | 1.19 | fixtures/call-analysis-go-project/go.mod |
+-------------------------------------+------+-----------+-----------------------------+---------+------------------------------------------+

---

[TestRunCallAnalysis/Run_with_govulncheck - 2]

---

[TestRun_GithubActions/scanning_osv-scanner_custom_format - 1]
Scanned <rootdir>/fixtures/locks-insecure/osv-scanner-flutter-deps.json file as a osv-scanner and found 3 packages
+--------------------------------+------+-----------+----------------------------+----------------------------+-------------------------------------------------------+
Expand Down
1 change: 1 addition & 0 deletions cmd/osv-scanner/fixtures/.goignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
call-analysis-go-project
9 changes: 9 additions & 0 deletions cmd/osv-scanner/fixtures/call-analysis-go-project/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module github.com/ossf-tests/osv-e2e

go 1.19

require github.com/gogo/protobuf v1.3.1

require github.com/ipfs/go-bitfield v1.0.0

require golang.org/x/image v0.4.0 // indirect
33 changes: 33 additions & 0 deletions cmd/osv-scanner/fixtures/call-analysis-go-project/go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
github.com/gogo/protobuf v1.3.1 h1:DqDEcV5aeaTmdFBePNpYsp3FlcVH/2ISVVM9Qf8PSls=
github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
github.com/ipfs/go-bitfield v1.0.0 h1:y/XHm2GEmD9wKngheWNNCNL0pzrWXZwCdQGv1ikXknQ=
github.com/ipfs/go-bitfield v1.0.0/go.mod h1:N/UiujQy+K+ceU1EF5EkVd1TNqevLrCQMIcAEPrdtus=
github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/image v0.4.0 h1:x1RWAiZIvERqkltrFjtQP1ycmiR5pmhjtCfVOtdURuQ=
golang.org/x/image v0.4.0/go.mod h1:FVC7BI/5Ym8R25iw5OLsgshdUBbT1h5jZTpA+mvAdZ4=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20181030221726-6c7e314b6563/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
24 changes: 24 additions & 0 deletions cmd/osv-scanner/fixtures/call-analysis-go-project/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package main

import (
"log"
"net/http"

"github.com/gogo/protobuf/plugin/unmarshal"
"github.com/gogo/protobuf/version"
"github.com/ipfs/go-bitfield"
)

func main() {
print(version.AtLeast("v1.2.3"))
unmarshal.NewUnmarshal()

bitfield.NewBitfield(14)

// Test stdlib
err := http.ListenAndServe(":8080", nil)
if err != nil {
log.Fatal(err)
}

}
24 changes: 24 additions & 0 deletions cmd/osv-scanner/fixtures/call-analysis-go-project/osv-scanner.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[[IgnoredVulns]]
id = "GO-2021-0053"
# ignoreUntil = n/a
reason = "This is a intentionally vulnerable test project"

[[IgnoredVulns]]
id = "GO-2023-1558"
# ignoreUntil = n/a
reason = "This is a intentionally vulnerable test project"

[[IgnoredVulns]]
id = "GO-2023-1572"
# ignoreUntil = n/a
reason = "This is a intentionally vulnerable test project"

[[IgnoredVulns]]
id = "GO-2023-1989"
# ignoreUntil = n/a
reason = "This is a intentionally vulnerable test project"

[[IgnoredVulns]]
id = "GO-2023-1990"
# ignoreUntil = n/a
reason = "This is a intentionally vulnerable test project"
26 changes: 26 additions & 0 deletions cmd/osv-scanner/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,32 @@ func TestRun(t *testing.T) {
}
}

func TestRunCallAnalysis(t *testing.T) {
t.Parallel()

// Switch to acceptance test if this takes too long, or when we add rust tests
// testutility.SkipIfNotAcceptanceTesting(t, "Takes a while to run")

tests := []cliTestCase{
{
name: "Run with govulncheck",
args: []string{"",
"--call-analysis=go",
"--config=./fixtures/osv-scanner-empty-config.toml",
"./fixtures/call-analysis-go-project"},
exit: 1,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

testCli(t, tt)
})
}
}

func TestRun_LockfileWithExplicitParseAs(t *testing.T) {
t.Parallel()

Expand Down
Loading

0 comments on commit a40c813

Please sign in to comment.