Skip to content

Commit 3072335

Browse files
author
Maceo Thompson
committed
internal/scan, vulncheck: use packages.load for mod info
Govulncheck previously used go list to get mod info, which does not work in modules with a vendor directory. Therefore, module information needs to be extracted from package information instead. There is one change to the behavior of govulncheck ran in module mode in a certain edge case: if one runs govulncheck with the ./... package pattern in a subdirectory of a module, govulncheck will only show the vulnerabilities affecting that subdirectory as opposed to the entire module. This does not affect govulncheck default behavior nor the behavior of govulncheck when ran from the root of a module at any scan level. Fixes golang/go#65124 Change-Id: Ie3b0cb0b9486fb94efeb05ee0c76d19c9f595877 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/557495 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
1 parent 0047a16 commit 3072335

File tree

10 files changed

+63
-133
lines changed

10 files changed

+63
-133
lines changed

cmd/govulncheck/testdata/testfiles/source-call/source_replace_text.ct

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Test of source mode on a module with a replace directive.
33

44
$ govulncheck -C ${moddir}/replace ./... --> FAIL 3
5-
Scanning your code and P packages across M dependent module for known vulnerabilities...
5+
Scanning your code and P packages across M dependent modules for known vulnerabilities...
66

77
Vulnerability #1: GO-2021-0113
88
Due to improper index calculation, an incorrectly formatted language tag can

cmd/govulncheck/testdata/testfiles/source-call/source_subdir_text.ct

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Vulnerability #1: GO-2021-0113
1717

1818
=== Informational ===
1919

20-
There are 4 vulnerabilities in modules that you require that are
20+
There are 2 vulnerabilities in modules that you require that are
2121
neither imported nor called. You may not need to take any action.
2222
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.
2323

@@ -30,24 +30,7 @@ Vulnerability #1: GO-2022-0969
3030
Found in: net/http@go1.18
3131
Fixed in: net/http@go1.18.6
3232

33-
Vulnerability #2: GO-2021-0265
34-
A maliciously crafted path can cause Get and other query functions to
35-
consume excessive amounts of CPU and time.
36-
More info: https://pkg.go.dev/vuln/GO-2021-0265
37-
Module: github.com/tidwall/gjson
38-
Found in: github.com/tidwall/gjson@v1.6.5
39-
Fixed in: github.com/tidwall/gjson@v1.9.3
40-
41-
Vulnerability #3: GO-2021-0054
42-
Due to improper bounds checking, maliciously crafted JSON objects can cause
43-
an out-of-bounds panic. If parsing user input, this may be used as a denial
44-
of service vector.
45-
More info: https://pkg.go.dev/vuln/GO-2021-0054
46-
Module: github.com/tidwall/gjson
47-
Found in: github.com/tidwall/gjson@v1.6.5
48-
Fixed in: github.com/tidwall/gjson@v1.6.6
49-
50-
Vulnerability #4: GO-2020-0015
33+
Vulnerability #2: GO-2020-0015
5134
An attacker could provide a single byte to a UTF16 decoder instantiated with
5235
UseBOM or ExpectBOM to trigger an infinite loop if the String function on
5336
the Decoder is called, or the Decoder is passed to transform.String. If used
@@ -83,7 +66,7 @@ Vulnerability #1: GO-2021-0113
8366

8467
=== Informational ===
8568

86-
There are 4 vulnerabilities in modules that you require that are
69+
There are 2 vulnerabilities in modules that you require that are
8770
neither imported nor called. You may not need to take any action.
8871
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.
8972

@@ -96,24 +79,7 @@ Vulnerability #1: GO-2022-0969
9679
Found in: net/http@go1.18
9780
Fixed in: net/http@go1.18.6
9881

99-
Vulnerability #2: GO-2021-0265
100-
A maliciously crafted path can cause Get and other query functions to
101-
consume excessive amounts of CPU and time.
102-
More info: https://pkg.go.dev/vuln/GO-2021-0265
103-
Module: github.com/tidwall/gjson
104-
Found in: github.com/tidwall/gjson@v1.6.5
105-
Fixed in: github.com/tidwall/gjson@v1.9.3
106-
107-
Vulnerability #3: GO-2021-0054
108-
Due to improper bounds checking, maliciously crafted JSON objects can cause
109-
an out-of-bounds panic. If parsing user input, this may be used as a denial
110-
of service vector.
111-
More info: https://pkg.go.dev/vuln/GO-2021-0054
112-
Module: github.com/tidwall/gjson
113-
Found in: github.com/tidwall/gjson@v1.6.5
114-
Fixed in: github.com/tidwall/gjson@v1.6.6
115-
116-
Vulnerability #4: GO-2020-0015
82+
Vulnerability #2: GO-2020-0015
11783
An attacker could provide a single byte to a UTF16 decoder instantiated with
11884
UseBOM or ExpectBOM to trigger an infinite loop if the String function on
11985
the Decoder is called, or the Decoder is passed to transform.String. If used

internal/scan/source.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,36 +30,29 @@ func runSource(ctx context.Context, handler govulncheck.Handler, cfg *config, cl
3030
return errNoGoMod
3131
}
3232
var pkgs []*packages.Package
33+
var mods []*packages.Module
3334
graph := vulncheck.NewPackageGraph(cfg.GoVersion)
3435
pkgConfig := &packages.Config{
3536
Dir: dir,
3637
Tests: cfg.test,
3738
Env: cfg.env,
3839
}
39-
mods, err := graph.LoadModules(pkgConfig)
40+
pkgs, mods, err = graph.LoadPackagesAndMods(pkgConfig, cfg.tags, cfg.patterns)
4041
if err != nil {
4142
if isGoVersionMismatchError(err) {
4243
return fmt.Errorf("%v\n\n%v", errGoVersionMismatch, err)
4344
}
44-
return fmt.Errorf("loading modules: %w", err)
45-
}
46-
if cfg.ScanLevel.WantPackages() {
47-
pkgs, err = graph.LoadPackages(pkgConfig, cfg.tags, cfg.patterns)
48-
if err != nil {
49-
if isGoVersionMismatchError(err) {
50-
return fmt.Errorf("%v\n\n%v", errGoVersionMismatch, err)
51-
}
52-
return fmt.Errorf("loading packages: %w", err)
53-
}
45+
return fmt.Errorf("loading packages: %w", err)
5446
}
47+
5548
if err := handler.Progress(sourceProgressMessage(pkgs, len(mods)-1, cfg.ScanLevel)); err != nil {
5649
return err
5750
}
5851

5952
if cfg.ScanLevel.WantPackages() && len(pkgs) == 0 {
6053
return nil // early exit
6154
}
62-
return vulncheck.Source(ctx, handler, pkgs, &cfg.Config, client, graph)
55+
return vulncheck.Source(ctx, handler, pkgs, mods, &cfg.Config, client, graph)
6356
}
6457

6558
// sourceProgressMessage returns a string of the form

internal/vulncheck/packages.go

Lines changed: 36 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,7 @@
55
package vulncheck
66

77
import (
8-
"bytes"
9-
"encoding/json"
10-
"errors"
118
"fmt"
12-
"os/exec"
13-
"path/filepath"
149
"strings"
1510

1611
"golang.org/x/tools/go/packages"
@@ -37,43 +32,6 @@ func NewPackageGraph(goVersion string) *PackageGraph {
3732
return graph
3833
}
3934

40-
func (g *PackageGraph) LoadModules(cfg *packages.Config) (mods []*packages.Module, err error) {
41-
cmd := exec.Command("go", "list", "-m", "-json")
42-
// Quick fix for go.dev/issue/65155
43-
// TODO: Fix go.dev/issue/65124
44-
// This check makes it so that govulncheck doesn't crash if running on a
45-
// vendored module from the root of a module. Essentially only here so that
46-
// the vendor test doesn't fail until #65124 is fixed.
47-
if fileExists(filepath.Join(cfg.Dir, "vendor")) {
48-
cmd.Args = append(cmd.Args, "-mod=readonly")
49-
}
50-
51-
cmd.Args = append(cmd.Args, "all")
52-
cmd.Env = append(cmd.Env, cfg.Env...)
53-
cmd.Dir = cfg.Dir
54-
out, err := cmd.Output()
55-
if err != nil {
56-
if ee := (*exec.ExitError)(nil); errors.As(err, &ee) && len(ee.Stderr) > 0 {
57-
return nil, fmt.Errorf("%v: %v\n%s", cmd, err, ee.Stderr)
58-
}
59-
return nil, fmt.Errorf("%v: %v", cmd, err)
60-
}
61-
62-
dec := json.NewDecoder(bytes.NewReader(out))
63-
for dec.More() {
64-
var m *packages.Module
65-
err = dec.Decode(&m)
66-
if err != nil {
67-
if err != nil {
68-
return nil, fmt.Errorf("decoding output of %v: %v", cmd, err)
69-
}
70-
}
71-
mods = append(mods, m)
72-
}
73-
g.AddModules(mods...)
74-
return mods, nil
75-
}
76-
7735
// AddModules adds the modules and any replace modules provided.
7836
// It will ignore modules that have duplicate paths to ones the graph already holds.
7937
func (g *PackageGraph) AddModules(mods ...*packages.Module) {
@@ -158,7 +116,7 @@ func (g *PackageGraph) GetPackage(path string) *packages.Package {
158116

159117
// LoadPackages loads the packages specified by the patterns into the graph.
160118
// See golang.org/x/tools/go/packages.Load for details of how it works.
161-
func (g *PackageGraph) LoadPackages(cfg *packages.Config, tags []string, patterns []string) ([]*packages.Package, error) {
119+
func (g *PackageGraph) LoadPackagesAndMods(cfg *packages.Config, tags []string, patterns []string) ([]*packages.Package, []*packages.Module, error) {
162120
if len(tags) > 0 {
163121
cfg.BuildFlags = []string{fmt.Sprintf("-tags=%s", strings.Join(tags, ","))}
164122
}
@@ -173,7 +131,7 @@ func (g *PackageGraph) LoadPackages(cfg *packages.Config, tags []string, pattern
173131

174132
pkgs, err := packages.Load(cfg, patterns...)
175133
if err != nil {
176-
return nil, err
134+
return nil, nil, err
177135
}
178136
var perrs []packages.Error
179137
packages.Visit(pkgs, nil, func(p *packages.Package) {
@@ -183,7 +141,40 @@ func (g *PackageGraph) LoadPackages(cfg *packages.Config, tags []string, pattern
183141
err = &packageError{perrs}
184142
}
185143
g.AddPackages(pkgs...)
186-
return pkgs, err
144+
return pkgs, extractModules(pkgs), err
145+
}
146+
147+
// extractModules collects modules in `pkgs` up to uniqueness of
148+
// module path and version.
149+
func extractModules(pkgs []*packages.Package) []*packages.Module {
150+
modMap := map[string]*packages.Module{}
151+
seen := map[*packages.Package]bool{}
152+
var extract func(*packages.Package, map[string]*packages.Module)
153+
extract = func(pkg *packages.Package, modMap map[string]*packages.Module) {
154+
if pkg == nil || seen[pkg] {
155+
return
156+
}
157+
if pkg.Module != nil {
158+
if pkg.Module.Replace != nil {
159+
modMap[pkg.Module.Replace.Path] = pkg.Module
160+
} else {
161+
modMap[pkg.Module.Path] = pkg.Module
162+
}
163+
}
164+
seen[pkg] = true
165+
for _, imp := range pkg.Imports {
166+
extract(imp, modMap)
167+
}
168+
}
169+
for _, pkg := range pkgs {
170+
extract(pkg, modMap)
171+
}
172+
173+
modules := []*packages.Module{}
174+
for _, mod := range modMap {
175+
modules = append(modules, mod)
176+
}
177+
return modules
187178
}
188179

189180
// packageError contains errors from loading a set of packages.

internal/vulncheck/slicing_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func Do(i I, input string) {
9191
})
9292

9393
graph := NewPackageGraph("go1.18")
94-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "/module/slice")})
94+
pkgs, _, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "/module/slice")})
9595
if err != nil {
9696
t.Fatal(err)
9797
}

internal/vulncheck/source.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import (
1717
)
1818

1919
// Source detects vulnerabilities in pkgs and emits the findings to handler.
20-
func Source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) error {
21-
vr, err := source(ctx, handler, pkgs, cfg, client, graph)
20+
func Source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, mods []*packages.Module, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) error {
21+
vr, err := source(ctx, handler, pkgs, mods, cfg, client, graph)
2222
if err != nil {
2323
return err
2424
}
@@ -33,7 +33,7 @@ func Source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
3333
// and produces a Result that contains info on detected vulnerabilities.
3434
//
3535
// Assumes that pkgs are non-empty and belong to the same program.
36-
func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) (*Result, error) {
36+
func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, mods []*packages.Module, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) (*Result, error) {
3737
ctx, cancel := context.WithCancel(ctx)
3838
defer cancel()
3939

@@ -57,10 +57,6 @@ func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
5757
}()
5858
}
5959

60-
var mods []*packages.Module
61-
for _, m := range graph.modules {
62-
mods = append(mods, m)
63-
}
6460
mv, err := FetchVulnerabilities(ctx, client, mods)
6561
if err != nil {
6662
return nil, err

internal/vulncheck/source_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func TestCalls(t *testing.T) {
191191

192192
// Load x and y as entry packages.
193193
graph := NewPackageGraph("go1.18")
194-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "entry/x"), path.Join(e.Temp(), "entry/y")})
194+
pkgs, mods, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "entry/x"), path.Join(e.Temp(), "entry/y")})
195195
if err != nil {
196196
t.Fatal(err)
197197
}
@@ -205,7 +205,7 @@ func TestCalls(t *testing.T) {
205205
}
206206

207207
cfg := &govulncheck.Config{ScanLevel: "symbol"}
208-
result, err := source(context.Background(), test.NewMockHandler(), pkgs, cfg, c, graph)
208+
result, err := source(context.Background(), test.NewMockHandler(), pkgs, mods, cfg, c, graph)
209209
if err != nil {
210210
t.Fatal(err)
211211
}
@@ -298,7 +298,7 @@ func TestAllSymbolsVulnerable(t *testing.T) {
298298

299299
// Load x as entry package.
300300
graph := NewPackageGraph("go1.18")
301-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
301+
pkgs, mods, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
302302
if err != nil {
303303
t.Fatal(err)
304304
}
@@ -307,7 +307,7 @@ func TestAllSymbolsVulnerable(t *testing.T) {
307307
}
308308

309309
cfg := &govulncheck.Config{ScanLevel: "symbol"}
310-
result, err := source(context.Background(), test.NewMockHandler(), pkgs, cfg, client, graph)
310+
result, err := source(context.Background(), test.NewMockHandler(), pkgs, mods, cfg, client, graph)
311311
if err != nil {
312312
t.Fatal(err)
313313
}
@@ -363,7 +363,7 @@ func TestNoSyntheticNodes(t *testing.T) {
363363

364364
// Load x as entry package.
365365
graph := NewPackageGraph("go1.18")
366-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
366+
pkgs, mods, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
367367
if err != nil {
368368
t.Fatal(err)
369369
}
@@ -377,7 +377,7 @@ func TestNoSyntheticNodes(t *testing.T) {
377377
}
378378

379379
cfg := &govulncheck.Config{ScanLevel: "symbol"}
380-
result, err := source(context.Background(), test.NewMockHandler(), pkgs, cfg, c, graph)
380+
result, err := source(context.Background(), test.NewMockHandler(), pkgs, mods, cfg, c, graph)
381381
if err != nil {
382382
t.Fatal(err)
383383
}
@@ -443,7 +443,7 @@ func TestRecursion(t *testing.T) {
443443

444444
// Load x as entry package.
445445
graph := NewPackageGraph("go1.18")
446-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
446+
pkgs, mods, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
447447
if err != nil {
448448
t.Fatal(err)
449449
}
@@ -457,7 +457,7 @@ func TestRecursion(t *testing.T) {
457457
}
458458

459459
cfg := &govulncheck.Config{ScanLevel: "symbol"}
460-
result, err := source(context.Background(), test.NewMockHandler(), pkgs, cfg, c, graph)
460+
result, err := source(context.Background(), test.NewMockHandler(), pkgs, mods, cfg, c, graph)
461461
if err != nil {
462462
t.Fatal(err)
463463
}
@@ -508,7 +508,7 @@ func TestIssue57174(t *testing.T) {
508508

509509
// Load x as entry package.
510510
graph := NewPackageGraph("go1.18")
511-
pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
511+
pkgs, mods, err := graph.LoadPackagesAndMods(e.Config, nil, []string{path.Join(e.Temp(), "entry/x")})
512512
if err != nil {
513513
t.Fatal(err)
514514
}
@@ -522,7 +522,7 @@ func TestIssue57174(t *testing.T) {
522522
}
523523

524524
cfg := &govulncheck.Config{ScanLevel: "symbol"}
525-
_, err = source(context.Background(), test.NewMockHandler(), pkgs, cfg, c, graph)
525+
_, err = source(context.Background(), test.NewMockHandler(), pkgs, mods, cfg, c, graph)
526526
if err != nil {
527527
t.Fatal(err)
528528
}

internal/vulncheck/utils.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@ package vulncheck
77
import (
88
"bytes"
99
"context"
10-
"errors"
1110
"go/token"
1211
"go/types"
13-
"os"
1412
"sort"
1513
"strings"
1614

@@ -359,17 +357,3 @@ func modVersion(mod *packages.Module) string {
359357
}
360358
return mod.Version
361359
}
362-
363-
// fileExists checks if file path exists. Returns true
364-
// if the file exists or it cannot prove that it does
365-
// not exist. Otherwise, returns false.
366-
func fileExists(path string) bool {
367-
if _, err := os.Stat(path); err == nil {
368-
return true
369-
} else if errors.Is(err, os.ErrNotExist) {
370-
return false
371-
}
372-
// Conservatively return true if os.Stat fails
373-
// for some other reason.
374-
return true
375-
}

0 commit comments

Comments
 (0)