Skip to content

Commit d51afa6

Browse files
author
Maceo Thompson
committed
cmd/govulncheck: fix mod level behavior
Fixes a bug where govulncheck would incorrectly state that there were no packages found matching the provided pattern when ran with the -scan=module flag. Additionally, this change adds base regression tests to ensure that: 1. Module level scanning short circuits before emitting more granular findings 2. Module level scanning still emits the same osvs as source level Adds tests to ensure that module level scans behave as expected. Further tests will be added for edge cases. Change-Id: I144a785c416501e84c0f089c40456cebd7738456 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/543162 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
1 parent 286bb05 commit d51afa6

File tree

3 files changed

+313
-21
lines changed

3 files changed

+313
-21
lines changed

‎cmd/govulncheck/testdata/testfiles/source-module/json/module_short_curcuits.ct

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
#####
2+
# Test that findings with callstacks or packages are not emitted in module mode
3+
$ govulncheck -json -scan module -C ${moddir}/multientry .
4+
{
5+
"config": {
6+
"protocol_version": "v1.0.0",
7+
"scanner_name": "govulncheck",
8+
"scanner_version": "v0.0.0-00000000000-20000101010101",
9+
"db": "testdata/vulndb-v1",
10+
"db_last_modified": "2023-04-03T15:57:51Z",
11+
"go_version": "go1.18",
12+
"scan_level": "module"
13+
}
14+
}
15+
{
16+
"progress": {
17+
"message": "Scanning your code across 2 dependent modules for known vulnerabilities..."
18+
}
19+
}
20+
{
21+
"osv": {
22+
"schema_version": "1.3.1",
23+
"id": "GO-2022-0969",
24+
"modified": "2023-04-03T15:57:51Z",
25+
"published": "2022-09-12T20:23:06Z",
26+
"aliases": [
27+
"CVE-2022-27664",
28+
"GHSA-69cg-p879-7622"
29+
],
30+
"details": "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.",
31+
"affected": [
32+
{
33+
"package": {
34+
"name": "stdlib",
35+
"ecosystem": "Go"
36+
},
37+
"ranges": [
38+
{
39+
"type": "SEMVER",
40+
"events": [
41+
{
42+
"introduced": "0"
43+
},
44+
{
45+
"fixed": "1.18.6"
46+
},
47+
{
48+
"introduced": "1.19.0"
49+
},
50+
{
51+
"fixed": "1.19.1"
52+
}
53+
]
54+
}
55+
],
56+
"ecosystem_specific": {
57+
"imports": [
58+
{
59+
"path": "net/http",
60+
"symbols": [
61+
"ListenAndServe",
62+
"ListenAndServeTLS",
63+
"Serve",
64+
"ServeTLS",
65+
"Server.ListenAndServe",
66+
"Server.ListenAndServeTLS",
67+
"Server.Serve",
68+
"Server.ServeTLS",
69+
"http2Server.ServeConn",
70+
"http2serverConn.goAway"
71+
]
72+
}
73+
]
74+
}
75+
},
76+
{
77+
"package": {
78+
"name": "golang.org/x/net",
79+
"ecosystem": "Go"
80+
},
81+
"ranges": [
82+
{
83+
"type": "SEMVER",
84+
"events": [
85+
{
86+
"introduced": "0"
87+
},
88+
{
89+
"fixed": "0.0.0-20220906165146-f3363e06e74c"
90+
}
91+
]
92+
}
93+
],
94+
"ecosystem_specific": {
95+
"imports": [
96+
{
97+
"path": "golang.org/x/net/http2",
98+
"symbols": [
99+
"Server.ServeConn",
100+
"serverConn.goAway"
101+
]
102+
}
103+
]
104+
}
105+
}
106+
],
107+
"references": [
108+
{
109+
"type": "WEB",
110+
"url": "https://groups.google.com/g/golang-announce/c/x49AQzIVX-s"
111+
},
112+
{
113+
"type": "REPORT",
114+
"url": "https://go.dev/issue/54658"
115+
},
116+
{
117+
"type": "FIX",
118+
"url": "https://go.dev/cl/428735"
119+
}
120+
],
121+
"credits": [
122+
{
123+
"name": "Bahruz Jabiyev, Tommaso Innocenti, Anthony Gavazzi, Steven Sprecher, and Kaan Onarlioglu"
124+
}
125+
],
126+
"database_specific": {
127+
"url": "https://pkg.go.dev/vuln/GO-2022-0969"
128+
}
129+
}
130+
}
131+
{
132+
"finding": {
133+
"osv": "GO-2022-0969",
134+
"fixed_version": "v1.18.6",
135+
"trace": [
136+
{
137+
"module": "stdlib",
138+
"version": "v1.18.0",
139+
"package": "net/http"
140+
}
141+
]
142+
}
143+
}
144+
{
145+
"osv": {
146+
"schema_version": "1.3.1",
147+
"id": "GO-2021-0113",
148+
"modified": "2023-04-03T15:57:51Z",
149+
"published": "2021-10-06T17:51:21Z",
150+
"aliases": [
151+
"CVE-2021-38561",
152+
"GHSA-ppp9-7jff-5vj2"
153+
],
154+
"details": "Due to improper index calculation, an incorrectly formatted language tag can cause Parse to panic via an out of bounds read. If Parse is used to process untrusted user inputs, this may be used as a vector for a denial of service attack.",
155+
"affected": [
156+
{
157+
"package": {
158+
"name": "golang.org/x/text",
159+
"ecosystem": "Go"
160+
},
161+
"ranges": [
162+
{
163+
"type": "SEMVER",
164+
"events": [
165+
{
166+
"introduced": "0"
167+
},
168+
{
169+
"fixed": "0.3.7"
170+
}
171+
]
172+
}
173+
],
174+
"ecosystem_specific": {
175+
"imports": [
176+
{
177+
"path": "golang.org/x/text/language",
178+
"symbols": [
179+
"MatchStrings",
180+
"MustParse",
181+
"Parse",
182+
"ParseAcceptLanguage"
183+
]
184+
}
185+
]
186+
}
187+
}
188+
],
189+
"references": [
190+
{
191+
"type": "FIX",
192+
"url": "https://go.dev/cl/340830"
193+
},
194+
{
195+
"type": "FIX",
196+
"url": "https://go.googlesource.com/text/+/383b2e75a7a4198c42f8f87833eefb772868a56f"
197+
}
198+
],
199+
"credits": [
200+
{
201+
"name": "Guido Vranken"
202+
}
203+
],
204+
"database_specific": {
205+
"url": "https://pkg.go.dev/vuln/GO-2021-0113"
206+
}
207+
}
208+
}
209+
{
210+
"finding": {
211+
"osv": "GO-2021-0113",
212+
"fixed_version": "v0.3.7",
213+
"trace": [
214+
{
215+
"module": "golang.org/x/text",
216+
"version": "v0.3.5"
217+
}
218+
]
219+
}
220+
}
221+
{
222+
"osv": {
223+
"schema_version": "1.3.1",
224+
"id": "GO-2020-0015",
225+
"modified": "2023-04-03T15:57:51Z",
226+
"published": "2021-04-14T20:04:52Z",
227+
"aliases": [
228+
"CVE-2020-14040",
229+
"GHSA-5rcv-m4m3-hfh7"
230+
],
231+
"details": "An attacker could provide a single byte to a UTF16 decoder instantiated with UseBOM or ExpectBOM to trigger an infinite loop if the String function on the Decoder is called, or the Decoder is passed to transform.String. If used to parse user supplied input, this may be used as a denial of service vector.",
232+
"affected": [
233+
{
234+
"package": {
235+
"name": "golang.org/x/text",
236+
"ecosystem": "Go"
237+
},
238+
"ranges": [
239+
{
240+
"type": "SEMVER",
241+
"events": [
242+
{
243+
"introduced": "0"
244+
},
245+
{
246+
"fixed": "0.3.3"
247+
}
248+
]
249+
}
250+
],
251+
"ecosystem_specific": {
252+
"imports": [
253+
{
254+
"path": "golang.org/x/text/encoding/unicode",
255+
"symbols": [
256+
"bomOverride.Transform",
257+
"utf16Decoder.Transform"
258+
]
259+
},
260+
{
261+
"path": "golang.org/x/text/transform",
262+
"symbols": [
263+
"String"
264+
]
265+
}
266+
]
267+
}
268+
}
269+
],
270+
"references": [
271+
{
272+
"type": "FIX",
273+
"url": "https://go.dev/cl/238238"
274+
},
275+
{
276+
"type": "FIX",
277+
"url": "https://go.googlesource.com/text/+/23ae387dee1f90d29a23c0e87ee0b46038fbed0e"
278+
},
279+
{
280+
"type": "REPORT",
281+
"url": "https://go.dev/issue/39491"
282+
},
283+
{
284+
"type": "WEB",
285+
"url": "https://groups.google.com/g/golang-announce/c/bXVeAmGOqz0"
286+
}
287+
],
288+
"credits": [
289+
{
290+
"name": "@abacabadabacaba and Anton Gyllenberg"
291+
}
292+
],
293+
"database_specific": {
294+
"url": "https://pkg.go.dev/vuln/GO-2020-0015"
295+
}
296+
}
297+
}

‎internal/scan/source.go

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ func runSource(ctx context.Context, handler govulncheck.Handler, cfg *config, cl
5252
return fmt.Errorf("loading packages: %w", err)
5353
}
5454
}
55-
if err := handler.Progress(sourceProgressMessage(pkgs, len(mods)-1)); err != nil {
55+
if err := handler.Progress(sourceProgressMessage(pkgs, len(mods)-1, cfg.ScanLevel)); err != nil {
5656
return err
5757
}
5858

59-
if len(pkgs) == 0 {
59+
if cfg.ScanLevel.WantPackages() && len(pkgs) == 0 {
6060
return nil // early exit
6161
}
6262
return vulncheck.Source(ctx, handler, pkgs, &cfg.Config, client, graph)
@@ -71,26 +71,21 @@ func runSource(ctx context.Context, handler govulncheck.Handler, cfg *config, cl
7171
// is 0, then the following message is returned
7272
//
7373
// "No packages matching the provided pattern."
74-
func sourceProgressMessage(topPkgs []*packages.Package, mods int) *govulncheck.Progress {
75-
if len(topPkgs) == 0 {
76-
// The package pattern is valid, but no packages are matching.
77-
// Example is pkg/strace/... (see #59623).
78-
return &govulncheck.Progress{Message: "No packages matching the provided pattern."}
79-
}
80-
81-
pkgs := depPkgs(topPkgs)
82-
83-
pkgsPhrase := fmt.Sprintf("%d package", pkgs)
84-
if pkgs != 1 {
85-
pkgsPhrase += "s"
86-
}
87-
88-
modsPhrase := fmt.Sprintf("%d dependent module", mods)
89-
if mods != 1 {
90-
modsPhrase += "s"
74+
func sourceProgressMessage(topPkgs []*packages.Package, mods int, mode govulncheck.ScanLevel) *govulncheck.Progress {
75+
var pkgsPhrase, modsPhrase string
76+
77+
if mode.WantPackages() {
78+
if len(topPkgs) == 0 {
79+
// The package pattern is valid, but no packages are matching.
80+
// Example is pkg/strace/... (see #59623).
81+
return &govulncheck.Progress{Message: "No packages matching the provided pattern."}
82+
}
83+
pkgs := depPkgs(topPkgs)
84+
pkgsPhrase = fmt.Sprintf(" and %d package%s", pkgs, choose(pkgs != 1, "s", ""))
9185
}
86+
modsPhrase = fmt.Sprintf(" %d dependent module%s", mods, choose(mods != 1, "s", ""))
9287

93-
msg := fmt.Sprintf("Scanning your code and %s across %s for known vulnerabilities...", pkgsPhrase, modsPhrase)
88+
msg := fmt.Sprintf("Scanning your code%s across%s for known vulnerabilities...", pkgsPhrase, modsPhrase)
9489
return &govulncheck.Progress{Message: msg}
9590
}
9691

‎internal/vulncheck/source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
3737
ctx, cancel := context.WithCancel(ctx)
3838
defer cancel()
3939

40-
fset := pkgs[0].Fset
4140
// If we are building the callgraph, build ssa and the callgraph in parallel
4241
// with fetching vulnerabilities. If the vulns set is empty, return without
4342
// waiting for SSA construction or callgraph to finish.
@@ -48,6 +47,7 @@ func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
4847
buildErr error
4948
)
5049
if cfg.ScanLevel.WantSymbols() {
50+
fset := pkgs[0].Fset
5151
wg.Add(1)
5252
go func() {
5353
defer wg.Done()

0 commit comments

Comments
 (0)