Skip to content

Commit 2992f25

Browse files
rolandshoemakerFiloSottile
rolandshoemaker
authored and
FiloSottile
committed
all: use the proxy for report linting
Check the proxy to determine valid versions and canonical module import paths. This should provent rogue database entries that do not cleanly apply to real go.mod files. Change-Id: Iea1b531fe5bed7a0825102c6ac877a515f24c0f5 Reviewed-on: https://team-review.git.corp.google.com/c/golang/vulndb/+/1032616 Reviewed-by: Roland Shoemaker <bracewell@google.com>
1 parent 4d3e0cc commit 2992f25

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+314
-185
lines changed

‎osv/json.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,14 @@ type Entry struct {
118118
}
119119

120120
func Generate(id string, url string, r report.Report) []Entry {
121+
importPath := r.Module
122+
if r.Package != "" {
123+
importPath = r.Package
124+
}
121125
entry := Entry{
122126
ID: id,
123127
Package: Package{
124-
Name: r.Package,
128+
Name: importPath,
125129
Ecosystem: GoEcosystem,
126130
},
127131
Summary: "", // TODO: think if we want to populate this in reports

‎osv/json_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@ import (
1010

1111
func TestGenerate(t *testing.T) {
1212
r := report.Report{
13-
Package: "example.com/vulnerable/v2",
13+
Module: "example.com/vulnerable/v2",
1414
AdditionalPackages: []struct {
15+
Module string
1516
Package string
1617
Symbols []string
1718
Versions []report.VersionRange
1819
}{
1920
{
20-
Package: "example.com/vulnerable",
21+
Module: "vanity.host/vulnerable",
22+
Package: "vanity.host/vulnerable/package",
2123
Symbols: []string{"b", "A.b"},
2224
Versions: []report.VersionRange{
2325
{Fixed: "v2.1.1"},
@@ -93,7 +95,7 @@ func TestGenerate(t *testing.T) {
9395

9496
ID: "GO-1991-0001",
9597
Package: Package{
96-
Name: "example.com/vulnerable",
98+
Name: "vanity.host/vulnerable/package",
9799
Ecosystem: "go",
98100
},
99101
Details: "It's a real bad one, I'll tell you that",

‎report/lint.go

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
package report
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"io"
7+
"net/http"
8+
"regexp"
9+
"strings"
10+
11+
"golang.org/x/mod/modfile"
12+
"golang.org/x/mod/module"
13+
"golang.org/x/mod/semver"
14+
)
15+
16+
// TODO: getting things from the proxy should all be cached so we
17+
// aren't re-requesting the same stuff over and over.
18+
19+
const proxyURL = "https://proxy.golang.org"
20+
21+
func getModVersions(module string) (map[string]bool, error) {
22+
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/list", proxyURL, module))
23+
if err != nil {
24+
return nil, err
25+
}
26+
defer resp.Body.Close()
27+
b, err := io.ReadAll(resp.Body)
28+
if err != nil {
29+
return nil, err
30+
}
31+
versions := map[string]bool{}
32+
for _, v := range strings.Split(string(b), "\n") {
33+
versions[v] = true
34+
}
35+
return versions, nil
36+
}
37+
38+
func getCanonicalModName(module string, version string) (string, error) {
39+
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/%s.mod", proxyURL, module, version))
40+
if err != nil {
41+
return "", err
42+
}
43+
defer resp.Body.Close()
44+
b, err := io.ReadAll(resp.Body)
45+
if err != nil {
46+
return "", err
47+
}
48+
m, err := modfile.ParseLax("go.mod", b, nil)
49+
if err != nil {
50+
return "", err
51+
}
52+
if m.Module == nil {
53+
return "", fmt.Errorf("unable to retrieve module information for %s", module)
54+
}
55+
return m.Module.Mod.Path, nil
56+
}
57+
58+
var pseudoVersionRE = regexp.MustCompile(`^v[0-9]+\.(0\.0-|\d+\.\d+-([^+]*\.)?0\.)\d{14}-[A-Za-z0-9]+(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$`)
59+
60+
// isPseudoVersion reports whether v is a pseudo-version.
61+
// NOTE: this is taken from cmd/go/internal/modfetch/pseudo.go but
62+
// uses regexp instead of the internal lazyregex package.
63+
func isPseudoVersion(v string) bool {
64+
return strings.Count(v, "-") >= 2 && semver.IsValid(v) && pseudoVersionRE.MatchString(v)
65+
}
66+
67+
func versionExists(version string, versions map[string]bool) error {
68+
// TODO: for now, just skip pseudo-versions. at some point we should verify that
69+
// it is a likely pseudo-version, i.e. one that could feasibly exist given the
70+
// actual versions that we know about.
71+
//
72+
// pseudo-version check should take into account the canonical import path
73+
// probably? (I think cmd/go/internal/modfetch/coderepo.go has something like
74+
// this, check the error containing "has post-%v module path")
75+
if isPseudoVersion(version) {
76+
return nil
77+
}
78+
if !versions[version] {
79+
return fmt.Errorf("proxy unaware of version")
80+
}
81+
return nil
82+
}
83+
84+
func checkModVersions(path string, vr []VersionRange) error {
85+
realVersions, err := getModVersions(path)
86+
if err != nil {
87+
return fmt.Errorf("unable to retrieve module versions from proxy: %s", err)
88+
}
89+
checkVersion := func(version string) error {
90+
if !semver.IsValid(version) {
91+
return errors.New("invalid module semver")
92+
}
93+
if err := module.Check(path, version); err != nil {
94+
return err
95+
}
96+
if err := versionExists(version, realVersions); err != nil {
97+
return err
98+
}
99+
canonicalPath, err := getCanonicalModName(path, version)
100+
if err != nil {
101+
return err
102+
}
103+
if canonicalPath != path {
104+
return fmt.Errorf("invalid module path at version (canonical path is %s)", canonicalPath)
105+
}
106+
return nil
107+
}
108+
for _, version := range vr {
109+
if version.Introduced != "" {
110+
if err := checkVersion(version.Introduced); err != nil {
111+
return fmt.Errorf("bad version.introduced %q: %s", version.Introduced, err)
112+
}
113+
}
114+
if version.Fixed != "" {
115+
if err := checkVersion(version.Fixed); err != nil {
116+
return fmt.Errorf("bad version.fixed %q: %s", version.Fixed, err)
117+
}
118+
}
119+
}
120+
return nil
121+
}
122+
123+
var cveRegex = regexp.MustCompile(`^CVE-\d{4}-\d{4,}$`)
124+
125+
func (vuln *Report) Lint() error {
126+
var importPath string
127+
if !vuln.Stdlib {
128+
if vuln.Module == "" {
129+
return errors.New("missing module")
130+
}
131+
if vuln.Package == vuln.Module {
132+
return errors.New("package is redundant and can be removed")
133+
}
134+
if vuln.Package != "" && !strings.HasPrefix(vuln.Package, vuln.Module) {
135+
return errors.New("module must be a prefix of package")
136+
}
137+
if vuln.Package == "" {
138+
importPath = vuln.Module
139+
} else {
140+
importPath = vuln.Package
141+
}
142+
if err := checkModVersions(vuln.Module, vuln.Versions); err != nil {
143+
return err
144+
}
145+
146+
if err := module.CheckImportPath(importPath); err != nil {
147+
return err
148+
}
149+
} else if vuln.Package == "" {
150+
return errors.New("missing package")
151+
}
152+
153+
for _, additionalPackage := range vuln.AdditionalPackages {
154+
var additionalImportPath string
155+
if additionalPackage.Module == "" {
156+
return errors.New("missing additional_package.module")
157+
}
158+
if additionalPackage.Package == additionalPackage.Module {
159+
return errors.New("package is redundant and can be removed")
160+
}
161+
if additionalPackage.Package != "" && !strings.HasPrefix(additionalPackage.Package, additionalPackage.Module) {
162+
return errors.New("additional_package.module must be a prefix of additional_package.package")
163+
}
164+
if additionalPackage.Package == "" {
165+
additionalImportPath = additionalPackage.Module
166+
} else {
167+
additionalImportPath = additionalPackage.Package
168+
}
169+
if err := module.CheckImportPath(additionalImportPath); err != nil {
170+
return err
171+
}
172+
if !vuln.Stdlib {
173+
if err := checkModVersions(additionalPackage.Module, additionalPackage.Versions); err != nil {
174+
return err
175+
}
176+
}
177+
}
178+
179+
if vuln.Description == "" {
180+
return errors.New("missing description")
181+
}
182+
183+
sevs := map[string]bool{
184+
"low": true,
185+
"medium": true,
186+
"high": true,
187+
"critical": true,
188+
}
189+
// Could also just default to medium if not provided?
190+
// Need to document what the default case is and what factors lower
191+
// or raise the sev
192+
if vuln.Severity != "" && !sevs[vuln.Severity] {
193+
return fmt.Errorf("unknown severity %q", vuln.Severity)
194+
}
195+
196+
if vuln.CVE != "" && vuln.CVEMetadata != nil && vuln.CVEMetadata.ID != "" {
197+
// TODO: may just want to use one of these? :shrug:
198+
return errors.New("only one of cve and cve_metadata.id should be present")
199+
}
200+
201+
if vuln.CVE != "" && !cveRegex.MatchString(vuln.CVE) {
202+
return fmt.Errorf("malformed CVE number: %s", vuln.CVE)
203+
}
204+
205+
return nil
206+
}

‎report/report.go

Lines changed: 2 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,12 @@
11
package report
22

3-
import (
4-
"errors"
5-
"fmt"
6-
"regexp"
7-
8-
"golang.org/x/mod/module"
9-
"golang.org/x/mod/semver"
10-
)
11-
123
type VersionRange struct {
134
Introduced string
145
Fixed string
156
}
167

178
type Report struct {
9+
Module string
1810
Package string
1911
// TODO: could also be GoToolchain, but we might want
2012
// this for other things?
@@ -29,6 +21,7 @@ type Report struct {
2921
// additional packages for some cases, but it's too heavy
3022
// for most
3123
AdditionalPackages []struct {
24+
Module string
3225
Package string
3326
Symbols []string
3427
Versions []VersionRange
@@ -52,67 +45,3 @@ type Report struct {
5245
Description string
5346
} `toml:"cve_metadata"`
5447
}
55-
56-
var cveRegex = regexp.MustCompile(`^CVE-\d{4}-\d{4,}$`)
57-
58-
func (vuln *Report) Lint() error {
59-
if vuln.Package == "" {
60-
return errors.New("missing package")
61-
}
62-
if err := module.CheckImportPath(vuln.Package); err != nil {
63-
return err
64-
}
65-
66-
for _, additionalPackage := range vuln.AdditionalPackages {
67-
if err := module.CheckImportPath(additionalPackage.Package); err != nil {
68-
return err
69-
}
70-
}
71-
72-
for _, version := range vuln.Versions {
73-
if version.Introduced != "" {
74-
if !semver.IsValid(version.Introduced) {
75-
return fmt.Errorf("bad version.introduced")
76-
}
77-
if err := module.Check(vuln.Package, version.Introduced); err != nil {
78-
return err
79-
}
80-
}
81-
if version.Fixed != "" {
82-
if !semver.IsValid(version.Fixed) {
83-
return fmt.Errorf("bad version.fixed")
84-
}
85-
if err := module.Check(vuln.Package, version.Fixed); err != nil {
86-
return err
87-
}
88-
}
89-
}
90-
91-
if vuln.Description == "" {
92-
return errors.New("missing description")
93-
}
94-
95-
sevs := map[string]bool{
96-
"low": true,
97-
"medium": true,
98-
"high": true,
99-
"critical": true,
100-
}
101-
// Could also just default to medium if not provided?
102-
// Need to document what the default case is and what factors lower
103-
// or raise the sev
104-
if vuln.Severity != "" && !sevs[vuln.Severity] {
105-
return fmt.Errorf("unknown severity %q", vuln.Severity)
106-
}
107-
108-
if vuln.CVE != "" && vuln.CVEMetadata != nil && vuln.CVEMetadata.ID != "" {
109-
// TODO: may just want to use one of these? :shrug:
110-
return errors.New("only one of cve and cve_metadata.id should be present")
111-
}
112-
113-
if vuln.CVE != "" && !cveRegex.MatchString(vuln.CVE) {
114-
return fmt.Errorf("malformed CVE number: %s", vuln.CVE)
115-
}
116-
117-
return nil
118-
}

‎reports/GO-2020-0001.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package = "github.com/gin-gonic/gin"
1+
module = "github.com/gin-gonic/gin"
22

33
description = """
44
The default [`Formatter`][LoggerConfig.Formatter] for the [`Logger`][] middleware
@@ -21,7 +21,7 @@ pr = "https://github.com/gin-gonic/gin/pull/2237"
2121
commit = "https://github.com/gin-gonic/gin/commit/a71af9c144f9579f6dbe945341c1df37aaf09c0d"
2222

2323
[cve_metadata]
24-
id = "CVE-XXX"
24+
id = "CVE-XXXX-0001"
2525
description = """
2626
Unsanitized input in the default logger in github.com/gin-gonic/gin before v1.6.0
2727
allows remote attackers to inject arbitary log lines.

‎reports/GO-2020-0002.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package = "github.com/proglottis/gpgme"
1+
module = "github.com/proglottis/gpgme"
22

33
cve = "CVE-2020-8945"
44

‎reports/GO-2020-0003.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package = "github.com/revel/revel"
1+
module = "github.com/revel/revel"
22

33
description = """
44
If the application accepts

‎reports/GO-2020-0004.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package = "github.com/nanobox-io/golang-nanoauth"
1+
module = "github.com/nanobox-io/golang-nanoauth"
22

33
description = """
44
If any of the `ListenAndServe` functions are called with an empty token,

‎reports/GO-2020-0005.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
package = "github.com/etcd-io/etcd/wal"
1+
module = "go.etcd.io/etcd"
2+
package = "go.etcd.io/etcd/wal"
23

34
description = """
45
Malformed WALs can be constructed such that [`WAL.ReadAll`][] can cause attempted

‎reports/GO-2020-0006.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package = "github.com/miekg/dns"
1+
module = "github.com/miekg/dns"
22

33
description = """
44
An attacker may prevent TCP connections to a [`Server`][] by opening

0 commit comments

Comments
 (0)