From 1468b95b49d724c7697627646efe79bf46f1fcbb Mon Sep 17 00:00:00 2001 From: Tatiana Bradley Date: Thu, 22 Jun 2023 17:25:14 -0400 Subject: [PATCH] internal/report,internal/proxy: add guessVulnerableAt Adds a new function, called by Fix, which attempts to guess an appropriate value for vulnerable_at by calling the module proxy. If this can't be determined, the field is left blank. Change-Id: Iceaf098c44ec5da23e7ea2156bc230431364dd10 Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/505298 Run-TryBot: Tatiana Bradley TryBot-Result: Gopher Robot Reviewed-by: Damien Neil --- internal/proxy/proxy.go | 71 +++++++++++++++++++++++++++++++--- internal/proxy/proxy_test.go | 74 +++++++++++++++++++++++++++++++++++- internal/report/fix.go | 63 ++++++++++++++++++++++++++++++ internal/report/fix_test.go | 55 +++++++++++++++++++++++++++ internal/report/ghsa_test.go | 23 +++++------ 5 files changed, 267 insertions(+), 19 deletions(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 4a37124d..f179ce59 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -12,10 +12,13 @@ import ( "net/http" "os" "path" + "sort" + "strings" "sync" "golang.org/x/mod/modfile" "golang.org/x/mod/module" + "golang.org/x/vulndb/internal/version" ) var DefaultClient *Client @@ -100,16 +103,44 @@ func (c *Client) CanonicalModulePath(path, version string) (_ string, err error) return m.Module.Mod.Path, nil } -func CanonicalModuleVersion(path, version string) (_ string, err error) { - return DefaultClient.CanonicalModuleVersion(path, version) +func CanonicalModuleVersion(path, ver string) (_ string, err error) { + return DefaultClient.CanonicalModuleVersion(path, ver) } -func (c *Client) CanonicalModuleVersion(path, version string) (_ string, err error) { +// CanonicalModuleVersion returns the canonical version string (with no leading "v" prefix) +// for the given module path and version string. +func (c *Client) CanonicalModuleVersion(path, ver string) (_ string, err error) { escaped, err := module.EscapePath(path) if err != nil { return "", err } - b, err := c.lookup(fmt.Sprintf("%s/@v/%v.info", escaped, version)) + b, err := c.lookup(fmt.Sprintf("%s/@v/%v.info", escaped, ver)) + if err != nil { + return "", err + } + var val map[string]any + if err := json.Unmarshal(b, &val); err != nil { + return "", err + } + v, ok := val["Version"].(string) + if !ok { + return "", fmt.Errorf("unable to retrieve canonical version for %s", ver) + } + return version.TrimPrefix(v), nil +} + +func Latest(path string) (string, error) { + return DefaultClient.Latest(path) +} + +// Latest returns the latest version of the module, with no leading "v" +// prefix. +func (c *Client) Latest(path string) (string, error) { + escaped, err := module.EscapePath(path) + if err != nil { + return "", err + } + b, err := c.lookup(fmt.Sprintf("%s/@latest", escaped)) if err != nil { return "", err } @@ -119,9 +150,37 @@ func (c *Client) CanonicalModuleVersion(path, version string) (_ string, err err } ver, ok := v["Version"].(string) if !ok { - return "", fmt.Errorf("unable to retrieve canonical version for %s", version) + return "", fmt.Errorf("unable to retrieve latest version for %s", path) + } + return version.TrimPrefix(ver), nil +} + +func Versions(path string) ([]string, error) { + return DefaultClient.Versions(path) +} + +// Versions returns a list of module versions (with no leading "v" prefix), +// sorted in ascending order. +func (c *Client) Versions(path string) ([]string, error) { + escaped, err := module.EscapePath(path) + if err != nil { + return nil, err + } + b, err := c.lookup(fmt.Sprintf("%s/@v/list", escaped)) + if err != nil { + return nil, err + } + if len(b) == 0 { + return nil, nil + } + var vs []string + for _, v := range strings.Split(strings.TrimSpace(string(b)), "\n") { + vs = append(vs, version.TrimPrefix(v)) } - return ver, nil + sort.SliceStable(vs, func(i, j int) bool { + return version.Before(vs[i], vs[j]) + }) + return vs, nil } func FindModule(path string) string { diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index a248f041..b7b1c4d2 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -10,6 +10,8 @@ import ( "net/http/httptest" "runtime" "testing" + + "github.com/google/go-cmp/cmp" ) func newTestClient(expectedEndpoint, mockResponse string) *Client { @@ -81,14 +83,14 @@ func TestCanonicalModuleVersion(t *testing.T) { path: "golang.org/x/vulndb", version: "v0.0.0-20230522180520-0cbf4ffdb4e7", mockResponse: `{"Version":"v0.0.0-20230522180520-0cbf4ffdb4e7"}`, - want: "v0.0.0-20230522180520-0cbf4ffdb4e7", + want: "0.0.0-20230522180520-0cbf4ffdb4e7", }, { name: "commit hash", path: "golang.org/x/vulndb", version: "0cbf4ffdb4e70fce663ec8d59198745b04e7801b", mockResponse: `{"Version":"v0.0.0-20230522180520-0cbf4ffdb4e7"}`, - want: "v0.0.0-20230522180520-0cbf4ffdb4e7", + want: "0.0.0-20230522180520-0cbf4ffdb4e7", }, } @@ -107,6 +109,74 @@ func TestCanonicalModuleVersion(t *testing.T) { } } +func TestVersions(t *testing.T) { + tcs := []struct { + name string + path string + mockResponse string + want []string + }{ + { + name: "no tagged versions", + path: "golang.org/x/vulndb", + mockResponse: "", + want: nil, + }, + { + name: "unsorted -> sorted", + path: "golang.org/x/tools", + mockResponse: ` +v0.1.4 +v0.9.3 +v0.7.0 +`, + want: []string{"0.1.4", "0.7.0", "0.9.3"}, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + endpoint := fmt.Sprintf("%s/@v/list", tc.path) + c := newTestClient(endpoint, tc.mockResponse) + got, err := c.Versions(tc.path) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("Versions() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestLatest(t *testing.T) { + tcs := []struct { + path string + mockResponse string + want string + }{ + { + path: "golang.org/x/vulndb", + mockResponse: `{"Version":"v0.0.0-20230522180520-0cbf4ffdb4e7"}`, + want: "0.0.0-20230522180520-0cbf4ffdb4e7", + }, + } + + for _, tc := range tcs { + t.Run(tc.path, func(t *testing.T) { + endpoint := fmt.Sprintf("%s/@latest", tc.path) + c := newTestClient(endpoint, tc.mockResponse) + got, err := c.Latest(tc.path) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Errorf("Latest() = %v, want %v", got, tc.want) + } + }) + } +} + func TestFindModule(t *testing.T) { tcs := []struct { name string diff --git a/internal/report/fix.go b/internal/report/fix.go index afb93556..7f579d2c 100644 --- a/internal/report/fix.go +++ b/internal/report/fix.go @@ -5,6 +5,8 @@ package report import ( + "errors" + "fmt" "regexp" "sort" "strings" @@ -89,6 +91,67 @@ func (m *Module) fixVersions() { } } } + + if m.VulnerableAt == "" { + v, err := m.guessVulnerableAt(proxy.DefaultClient) + // For now, ignore errors and leave the field blank if it can't + // be determined. + if err == nil { + m.VulnerableAt = v + } + } +} + +type proxyClient interface { + Latest(path string) (string, error) + Versions(path string) ([]string, error) +} + +// guessVulnerableAt attempts to find a vulnerable_at +// version using the module proxy. +// If there is no fix, the latest version is used. +func (m *Module) guessVulnerableAt(pc proxyClient) (string, error) { + if m.IsFirstParty() { + return "", errors.New("cannot auto-guess vulnerable_at for first-party modules") + } + + // Find the last fixed version, assuming the version ranges are sorted. + fixed := "" + if len(m.Versions) > 0 { + fixed = m.Versions[len(m.Versions)-1].Fixed + } + + // If there is no fix, find the latest version of the module. + if fixed == "" { + latest, err := pc.Latest(m.Module) + if err != nil || latest == "" { + return "", fmt.Errorf("could not find latest version from proxy: %s", err) + } + + return latest, nil + } + + // If the latest fixed version is a 0.0.0 pseudo-version, or not a valid version, + // don't attempt to determine the vulnerable_at version. + if !version.IsValid(fixed) { + return "", errors.New("cannot auto-guess when fixed version is invalid") + } + if strings.HasPrefix(fixed, "0.0.0-") { + return "", errors.New("cannot auto-guess when fixed version is 0.0.0 pseudo-version") + } + + // Otherwise, find the version right before the fixed version. + vs, err := pc.Versions(m.Module) + if err != nil { + return "", fmt.Errorf("could not find versions from proxy: %s", err) + } + for i := len(vs) - 1; i >= 0; i-- { + if version.Before(vs[i], fixed) { + return vs[i], nil + } + } + + return "", fmt.Errorf("could not find tagged version less than fixed (lowest version is %s)", vs[0]) } // fixLineLength returns a copy of s with all lines trimmed to <=n characters diff --git a/internal/report/fix_test.go b/internal/report/fix_test.go index 65bd5c82..d3ce2bff 100644 --- a/internal/report/fix_test.go +++ b/internal/report/fix_test.go @@ -5,6 +5,7 @@ package report import ( + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -173,3 +174,57 @@ broken up }) } } + +func TestGuessVulnerableAt(t *testing.T) { + for _, tc := range []struct { + name string + m *Module + want string + }{ + { + name: "no fix", + m: &Module{ + Module: "golang.org/x/tools", + }, + want: "0.2.0", + }, + { + name: "has fix", + m: &Module{ + Module: "golang.org/x/tools", + Versions: []VersionRange{ + { + Fixed: "0.1.8", + }, + }, + }, + want: "0.1.7", + }, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := tc.m.guessVulnerableAt(&mockProxy{}) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Errorf("guessVulnerableAt() = %q, want %s", got, tc.want) + } + }) + } +} + +type mockProxy struct{} + +func (m *mockProxy) Versions(path string) ([]string, error) { + if path == "golang.org/x/tools" { + return []string{"0.1.6", "0.1.7", "0.1.8", "0.2.0"}, nil + } + return nil, errors.New("unexpected input") +} + +func (m *mockProxy) Latest(path string) (string, error) { + if path == "golang.org/x/tools" { + return "0.2.0", nil + } + return "", errors.New("unexpected input") +} diff --git a/internal/report/ghsa_test.go b/internal/report/ghsa_test.go index 8137bb08..b1637166 100644 --- a/internal/report/ghsa_test.go +++ b/internal/report/ghsa_test.go @@ -21,9 +21,9 @@ func TestGHSAToReport(t *testing.T) { Permalink: "https://github.com/permalink/to/G1", Description: "a description", Vulns: []*ghsa.Vuln{{ - Package: "aPackage", - EarliestFixedVersion: "1.2.3", - VulnerableVersionRange: "< 1.2.3", + Package: "golang.org/x/tools/go/packages", + EarliestFixedVersion: "0.9.0", + VulnerableVersionRange: "< 0.9.0", }}, References: []ghsa.Reference{{URL: "https://github.com/permalink/to/issue/12345"}}, } @@ -34,15 +34,16 @@ func TestGHSAToReport(t *testing.T) { }{ { name: "module provided", - module: "aModule", + module: "golang.org/x/tools", want: &Report{ Modules: []*Module{{ - Module: "aModule", + Module: "golang.org/x/tools", + VulnerableAt: "0.8.0", Versions: []VersionRange{ - {Fixed: "1.2.3"}, + {Fixed: "0.9.0"}, }, Packages: []*Package{{ - Package: "aPackage", + Package: "golang.org/x/tools/go/packages", }}, }}, Description: "a description", @@ -52,16 +53,16 @@ func TestGHSAToReport(t *testing.T) { }, }, { - name: "empty module uses package", + name: "empty module uses package as module", module: "", want: &Report{ Modules: []*Module{{ - Module: "aPackage", + Module: "golang.org/x/tools/go/packages", Versions: []VersionRange{ - {Fixed: "1.2.3"}, + {Fixed: "0.9.0"}, }, Packages: []*Package{{ - Package: "aPackage", + Package: "golang.org/x/tools/go/packages", }}, }}, Description: "a description",