Skip to content

Commit

Permalink
internal: properly fetch modules in source mode
Browse files Browse the repository at this point in the history 
Loads modules using go list instead of inferring through packages. This
fixes a few module-level edge cases where a vuln wouldn't be counted.

Change-Id: I24e0ffa73f47451806d88aa672ca8ef7a72fc2bb
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/529278
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Zvonimir Pavlinovic <[email protected]>
  • Loading branch information
Maceo Thompson committed Oct 31, 2023
1 parent cc39747 commit 61b4508
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 66 deletions.
2 changes: 1 addition & 1 deletion cmd/govulncheck/testdata/source_multientry_json.ct
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ $ govulncheck -json -C ${moddir}/multientry .
}
{
"progress": {
"message": "Scanning your code and P packages across M dependent module for known vulnerabilities..."
"message": "Scanning your code and P packages across M dependent modules for known vulnerabilities..."
}
}
{
Expand Down
4 changes: 2 additions & 2 deletions cmd/govulncheck/testdata/source_multientry_text.ct
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#####
# Test for multiple call stacks in source mode with expanded traces
$ govulncheck -C ${moddir}/multientry . --> FAIL 3
Scanning your code and P packages across M dependent module for known vulnerabilities...
Scanning your code and P packages across M dependent modules for known vulnerabilities...

Vulnerability #1: GO-2021-0113
Due to improper index calculation, an incorrectly formatted language tag can
Expand Down Expand Up @@ -40,7 +40,7 @@ Share feedback at https://go.dev/s/govulncheck-feedback.
#####
# Test for multple call stacks in source mode with expanded traces
$ govulncheck -C ${moddir}/multientry -show=traces ./... --> FAIL 3
Scanning your code and P packages across M dependent module for known vulnerabilities...
Scanning your code and P packages across M dependent modules for known vulnerabilities...

Vulnerability #1: GO-2021-0113
Due to improper index calculation, an incorrectly formatted language tag can
Expand Down
46 changes: 40 additions & 6 deletions cmd/govulncheck/testdata/source_subdir_text.ct
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#####
# Test govulncheck runs on the subdirectory of a module
$ govulncheck -C ${moddir}/vuln/subdir . --> FAIL 3
Scanning your code and P packages across M dependent module for known vulnerabilities...
Scanning your code and P packages across M dependent modules for known vulnerabilities...

Vulnerability #1: GO-2021-0113
Due to improper index calculation, an incorrectly formatted language tag can
Expand All @@ -19,7 +19,7 @@ Vulnerability #1: GO-2021-0113

Found 0 vulnerabilities in packages that you import, but there are no
call stacks leading to the use of these vulnerabilities. You may not
need to take any action. There are also 2 vulnerabilities in modules
need to take any action. There are also 4 vulnerabilities in modules
that you require that are neither imported nor called.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Expand All @@ -32,7 +32,24 @@ Vulnerability #1: GO-2022-0969
Found in: net/[email protected]
Fixed in: net/[email protected]

Vulnerability #2: GO-2020-0015
Vulnerability #2: GO-2021-0265
A maliciously crafted path can cause Get and other query functions to
consume excessive amounts of CPU and time.
More info: https://pkg.go.dev/vuln/GO-2021-0265
Module: github.com/tidwall/gjson
Found in: github.com/tidwall/[email protected]
Fixed in: github.com/tidwall/[email protected]

Vulnerability #3: GO-2021-0054
Due to improper bounds checking, maliciously crafted JSON objects can cause
an out-of-bounds panic. If parsing user input, this may be used as a denial
of service vector.
More info: https://pkg.go.dev/vuln/GO-2021-0054
Module: github.com/tidwall/gjson
Found in: github.com/tidwall/[email protected]
Fixed in: github.com/tidwall/[email protected]

Vulnerability #4: GO-2020-0015
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
Expand All @@ -50,7 +67,7 @@ Share feedback at https://go.dev/s/govulncheck-feedback.
#####
# Test govulncheck runs on the subdirectory of a module
$ govulncheck -C ${moddir}/vuln/subdir -show=traces . --> FAIL 3
Scanning your code and P packages across M dependent module for known vulnerabilities...
Scanning your code and P packages across M dependent modules for known vulnerabilities...

Vulnerability #1: GO-2021-0113
Due to improper index calculation, an incorrectly formatted language tag can
Expand All @@ -70,7 +87,7 @@ Vulnerability #1: GO-2021-0113

Found 0 vulnerabilities in packages that you import, but there are no
call stacks leading to the use of these vulnerabilities. You may not
need to take any action. There are also 2 vulnerabilities in modules
need to take any action. There are also 4 vulnerabilities in modules
that you require that are neither imported nor called.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Expand All @@ -83,7 +100,24 @@ Vulnerability #1: GO-2022-0969
Found in: net/[email protected]
Fixed in: net/[email protected]

Vulnerability #2: GO-2020-0015
Vulnerability #2: GO-2021-0265
A maliciously crafted path can cause Get and other query functions to
consume excessive amounts of CPU and time.
More info: https://pkg.go.dev/vuln/GO-2021-0265
Module: github.com/tidwall/gjson
Found in: github.com/tidwall/[email protected]
Fixed in: github.com/tidwall/[email protected]

Vulnerability #3: GO-2021-0054
Due to improper bounds checking, maliciously crafted JSON objects can cause
an out-of-bounds panic. If parsing user input, this may be used as a denial
of service vector.
More info: https://pkg.go.dev/vuln/GO-2021-0054
Module: github.com/tidwall/gjson
Found in: github.com/tidwall/[email protected]
Fixed in: github.com/tidwall/[email protected]

Vulnerability #4: GO-2020-0015
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
Expand Down
52 changes: 30 additions & 22 deletions internal/scan/source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"sort"

"golang.org/x/tools/go/packages"
"golang.org/x/vuln/internal"
"golang.org/x/vuln/internal/client"
"golang.org/x/vuln/internal/govulncheck"
"golang.org/x/vuln/internal/vulncheck"
Expand All @@ -33,18 +32,17 @@ func runSource(ctx context.Context, handler govulncheck.Handler, cfg *config, cl
Tests: cfg.test,
Env: cfg.env,
}
pkgs, err := graph.LoadPackages(pkgConfig, cfg.tags, cfg.patterns)
mods, err := graph.LoadModules(pkgConfig)
if err != nil {
// Try to provide a meaningful and actionable error message.
if !fileExists(filepath.Join(dir, "go.mod")) {
return fmt.Errorf("govulncheck: %v", errNoGoMod)
}
if isGoVersionMismatchError(err) {
return fmt.Errorf("govulncheck: %v\n\n%v", errGoVersionMismatch, err)
return parseLoadError(err, dir, false)
}
if cfg.ScanLevel.WantPackages() {
pkgs, err = graph.LoadPackages(pkgConfig, cfg.tags, cfg.patterns)
if err != nil {
return parseLoadError(err, dir, true)
}
return fmt.Errorf("govulncheck: loading packages: %w", err)
}
if err := handler.Progress(sourceProgressMessage(pkgs)); err != nil {
if err := handler.Progress(sourceProgressMessage(pkgs, len(mods)-1)); err != nil {
return err
}

Expand Down Expand Up @@ -133,14 +131,14 @@ func frameFromPackage(pkg *packages.Package) *govulncheck.Frame {
// is 0, then the following message is returned
//
// "No packages matching the provided pattern."
func sourceProgressMessage(topPkgs []*packages.Package) *govulncheck.Progress {
func sourceProgressMessage(topPkgs []*packages.Package, mods int) *govulncheck.Progress {
if len(topPkgs) == 0 {
// The package pattern is valid, but no packages are matching.
// Example is pkg/strace/... (see #59623).
return &govulncheck.Progress{Message: "No packages matching the provided pattern."}
}

pkgs, mods := depPkgsAndMods(topPkgs)
pkgs := depPkgs(topPkgs)

pkgsPhrase := fmt.Sprintf("%d package", pkgs)
if pkgs != 1 {
Expand All @@ -156,12 +154,10 @@ func sourceProgressMessage(topPkgs []*packages.Package) *govulncheck.Progress {
return &govulncheck.Progress{Message: msg}
}

// depPkgsAndMods returns the number of packages that
// topPkgs depend on and the number of their modules.
func depPkgsAndMods(topPkgs []*packages.Package) (int, int) {
// depPkgs returns the number of packages that topPkgs depend on
func depPkgs(topPkgs []*packages.Package) int {
tops := make(map[string]bool)
depPkgs := make(map[string]bool)
depMods := make(map[string]bool)

for _, t := range topPkgs {
tops[t.PkgPath] = true
Expand All @@ -185,11 +181,6 @@ func depPkgsAndMods(topPkgs []*packages.Package) (int, int) {
// as a dependent package.
if !tops[path] {
depPkgs[path] = true
if p.Module != nil &&
p.Module.Path != internal.GoStdModulePath && // no module for stdlib
p.Module.Path != internal.UnknownModulePath { // no module for unknown
depMods[p.Module.Path] = true
}
}

for _, d := range p.Imports {
Expand All @@ -201,5 +192,22 @@ func depPkgsAndMods(topPkgs []*packages.Package) (int, int) {
visit(t, true)
}

return len(depPkgs), len(depMods)
return len(depPkgs)
}

// parseLoadError takes a non-nil error and tries to provide a meaningful
// and actionable error message to surface for the end user.
func parseLoadError(err error, dir string, pkgs bool) error {
if !fileExists(filepath.Join(dir, "go.mod")) {
return fmt.Errorf("govulncheck: %v", errNoGoMod)
}
if isGoVersionMismatchError(err) {
return fmt.Errorf("govulncheck: %v\n\n%v", errGoVersionMismatch, err)
}

level := "modules"
if pkgs {
level = "packages"
}
return fmt.Errorf("govulncheck: loading %s: %w", level, err)
}
98 changes: 98 additions & 0 deletions internal/scan/testdata/multi-stack-modlevel.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
{
"config": {
"protocol_version": "v0.1.0",
"scanner_name": "govulncheck"
}
}
{
"osv": {
"id": "GO-0000-0001",
"modified": "0001-01-01T00:00:00Z",
"published": "0001-01-01T00:00:00Z",
"details": "Third-party vulnerability",
"affected": [
{
"package": {
"name": "golang.org/vmod",
"ecosystem": ""
},
"ecosystem_specific": {
"imports": [
{
"goos": [
"amd"
]
}
]
}
}
],
"database_specific": {
"url": "https://pkg.go.dev/vuln/GO-0000-0001"
}
}
}
{
"finding": {
"osv": "GO-0000-0001",
"fixed_version": "v0.1.3",
"trace": [
{
"module": "golang.org/vmod",
"version": "v0.0.1"
}
]
}
}
{
"finding": {
"osv": "GO-0000-0001",
"fixed_version": "v0.1.3",
"trace": [
{
"module": "golang.org/vmod",
"version": "v0.0.1",
"package": "vmod",
"function": "VulnFoo"
},
{
"module": "golang.org/main",
"version": "v0.0.1",
"package": "main",
"function": "main"
}
]
}
}
{
"osv": {
"id": "GO-0000-0002",
"modified": "0001-01-01T00:00:00Z",
"published": "0001-01-01T00:00:00Z",
"details": "Third-party vulnerability",
"affected": [
{
"package": {
"name": "golang.org/vmod",
"ecosystem": ""
},
"ecosystem_specific": {}
}
],
"database_specific": {
"url": "https://pkg.go.dev/vuln/GO-0000-0002"
}
}
}
{
"finding": {
"osv": "GO-0000-0002",
"fixed_version": "v0.1.3",
"trace": [
{
"module": "golang.org/vmod",
"version": "v0.0.1"
}
]
}
}
28 changes: 28 additions & 0 deletions internal/scan/testdata/multi-stack-modlevel.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
Vulnerability #1: GO-0000-0001
Third-party vulnerability
More info: https://pkg.go.dev/vuln/GO-0000-0001
Module: golang.org/vmod
Found in: golang.org/[email protected]
Fixed in: golang.org/[email protected]
Platforms: amd
Example traces found:
#1: main.main calls vmod.VulnFoo

=== Informational ===

Found 0 vulnerabilities in packages that you import, but there are no
call stacks leading to the use of these vulnerabilities. You may not
need to take any action. There is also 1 vulnerability in modules
that you require that is neither imported nor called.
See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck for details.

Vulnerability #1: GO-0000-0002
Third-party vulnerability
More info: https://pkg.go.dev/vuln/GO-0000-0002
Module: golang.org/vmod
Found in: golang.org/[email protected]
Fixed in: golang.org/[email protected]

Your code is affected by 1 vulnerability from 1 module.

Share feedback at https://go.dev/s/govulncheck-feedback.
3 changes: 2 additions & 1 deletion internal/vulncheck/packages.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ func NewPackageGraph(goVersion string) *PackageGraph {
}

func (g *PackageGraph) LoadModules(cfg *packages.Config) (mods []*packages.Module, err error) {
cmd := exec.Command("go", "list", "-m", "-json", "all")
cmd := exec.Command("go", "list", "-m", "-json", "-mod=mod", "all")
cmd.Env = append(cmd.Env, cfg.Env...)
cmd.Dir = cfg.Dir
out, err := cmd.Output()
if err != nil {
return nil, err
}

dec := json.NewDecoder(bytes.NewReader(out))
for dec.More() {
var m *packages.Module
Expand Down
Loading

0 comments on commit 61b4508

Please sign in to comment.