internal/report: FixLink to FixLinks (a slice)
Changes FixLink in the Module struct to FixLinks, a slice of strings.
Change-Id: I0f6e2d8c1184aaf4da145088d996fb116b056099
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/562235
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/data/reports/GO-2024-2497.yaml b/data/reports/GO-2024-2497.yaml
index 61387a2..615cdb3 100644
--- a/data/reports/GO-2024-2497.yaml
+++ b/data/reports/GO-2024-2497.yaml
@@ -30,7 +30,8 @@
- gatewayFrontend.Solve
- NewBridgeForwarder
- llbBridgeForwarder.NewContainer
- fix_link: https://github.com/moby/buildkit/commit/5026d95aa3336e97cfe46e3764f52d08bac7a10e
+ fix_links:
+ - https://github.com/moby/buildkit/commit/5026d95aa3336e97cfe46e3764f52d08bac7a10e
summary: Privilege escalation in github.com/moby/buildkit
description: |-
BuildKit provides APIs for running interactive containers based on built images.
diff --git a/internal/report/report.go b/internal/report/report.go
index f40b76b..f29800c 100644
--- a/internal/report/report.go
+++ b/internal/report/report.go
@@ -56,7 +56,7 @@
Packages []*Package `yaml:",omitempty"`
// Used to automatically determine vulnerable symbols for a given module.
// Will be auto-populated from the reports "fix" link if none is specified.
- FixLink string `yaml:"fix_link,omitempty"`
+ FixLinks []string `yaml:"fix_links,omitempty"`
}
type Package struct {
@@ -270,6 +270,18 @@
return pkgs
}
+// CommitLinks returns all commit fix links in report.References
+func (r *Report) CommitLinks() (links []string) {
+ for _, ref := range r.References {
+ if ref.Type == osv.ReferenceTypeFix {
+ if strings.Contains(ref.URL, "commit") {
+ links = append(links, ref.URL)
+ }
+ }
+ }
+ return links
+}
+
// Aliases returns all aliases (e.g., CVEs, GHSAs) for a report.
func (r *Report) Aliases() []string {
return append(r.AllCVEs(), r.GHSAs...)
diff --git a/internal/symbols/populate.go b/internal/symbols/populate.go
index fedcf70..c10e88a 100644
--- a/internal/symbols/populate.go
+++ b/internal/symbols/populate.go
@@ -10,7 +10,7 @@
"path/filepath"
"strings"
- "golang.org/x/vulndb/internal/osv"
+ "golang.org/x/exp/slices"
"golang.org/x/vulndb/internal/report"
)
@@ -21,67 +21,67 @@
}
func populate(r *report.Report, patched func(string, string, string) (map[string][]string, error)) error {
- var defaultFixes []string
-
- for _, ref := range r.References {
- if ref.Type == osv.ReferenceTypeFix {
- if filepath.Base(filepath.Dir(ref.URL)) == "commit" {
- defaultFixes = append(defaultFixes, ref.URL)
- }
- }
- }
- if len(defaultFixes) == 0 {
- return fmt.Errorf("no commit fix links found")
- }
var errs []error
for _, mod := range r.Modules {
- hasFixLink := mod.FixLink != ""
- if hasFixLink {
- defaultFixes = append(defaultFixes, mod.FixLink)
- }
- numFixedSymbols := make([]int, len(defaultFixes))
- for i, fixLink := range defaultFixes {
- fixHash := filepath.Base(fixLink)
- fixRepo := strings.TrimSuffix(fixLink, "/commit/"+fixHash)
- pkgsToSymbols, err := patched(mod.Module, fixRepo, fixHash)
- if err != nil {
- errs = append(errs, err)
+ hasFixLink := len(mod.FixLinks) >= 0
+ fixLinks := mod.FixLinks
+ if len(fixLinks) == 0 {
+ c := r.CommitLinks()
+ if len(c) == 0 {
+ errs = append(errs, fmt.Errorf("no commit fix links found for module %s", mod.Module))
continue
}
- packages := mod.AllPackages()
- for pkg, symbols := range pkgsToSymbols {
- if _, exists := packages[pkg]; exists {
- packages[pkg].Symbols = append(packages[pkg].Symbols, symbols...)
- } else {
- mod.Packages = append(mod.Packages, &report.Package{
- Package: pkg,
- Symbols: symbols,
- })
- }
- numFixedSymbols[i] += len(symbols)
- }
+ fixLinks = c
}
- // if the module's link field wasn't already populated, populate it with
- // the link that results in the most symbols
- if hasFixLink {
- defaultFixes = defaultFixes[:len(defaultFixes)-1]
- } else {
- mod.FixLink = defaultFixes[indexMax(numFixedSymbols)]
+
+ foundSymbols := false
+ for _, fixLink := range fixLinks {
+ found, err := populateFromFixLink(fixLink, mod, patched)
+ if err != nil {
+ errs = append(errs, err)
+ }
+ foundSymbols = foundSymbols || found
+ }
+ if !foundSymbols && fixLinks != nil {
+ errs = append(errs, fmt.Errorf("no vulnerable symbols found for module %s", mod.Module))
+ }
+ // Sort fix links for testing/deterministic output
+ if !hasFixLink {
+ slices.Sort(mod.FixLinks)
}
}
return errors.Join(errs...)
}
-// indexMax takes a slice of nonempty ints and returns the index of the maximum value
-func indexMax(s []int) (index int) {
- maxVal := s[0]
- index = 0
- for i, val := range s {
- if val > maxVal {
- maxVal = val
- index = i
+// populateFromFixLink takes a fixLink and a module and returns true if any symbols
+// are found for the given fix/module pair.
+func populateFromFixLink(fixLink string, m *report.Module, patched func(string, string, string) (map[string][]string, error)) (foundSymbols bool, err error) {
+ fixHash := filepath.Base(fixLink)
+ fixRepo := strings.TrimSuffix(fixLink, "/commit/"+fixHash)
+ pkgsToSymbols, err := patched(m.Module, fixRepo, fixHash)
+ if err != nil {
+ return false, err
+ }
+ modPkgs := m.AllPackages()
+ for pkg, symbols := range pkgsToSymbols {
+ foundSymbols = true
+ if modPkg, exists := modPkgs[pkg]; exists {
+ // Ensure there are no duplicate symbols
+ for _, s := range symbols {
+ if !slices.Contains(modPkg.Symbols, s) {
+ modPkg.Symbols = append(modPkg.Symbols, s)
+ }
+ }
+ } else {
+ m.Packages = append(m.Packages, &report.Package{
+ Package: pkg,
+ Symbols: symbols,
+ })
}
}
- return index
+ if foundSymbols {
+ m.FixLinks = append(m.FixLinks, fixLink)
+ }
+ return foundSymbols, nil
}
diff --git a/internal/symbols/populate_test.go b/internal/symbols/populate_test.go
index 57e15e0..46528a8 100644
--- a/internal/symbols/populate_test.go
+++ b/internal/symbols/populate_test.go
@@ -37,7 +37,7 @@
Package: "example.com/module/package",
Symbols: []string{"symbol1", "symbol2"},
}},
- FixLink: "https://example.com/module/commit/1234",
+ FixLinks: []string{"https://example.com/module/commit/1234"},
}},
References: []*report.Reference{
{
@@ -70,10 +70,10 @@
Packages: []*report.Package{{
Package: "example.com/module/package",
// We don't yet dedupe the symbols.
- Symbols: []string{"symbol1", "symbol2", "symbol1", "symbol2", "symbol3"},
+ Symbols: []string{"symbol1", "symbol2", "symbol3"},
}},
- // This commit is picked because it results in the most symbols.
- FixLink: "https://example.com/module/commit/5678",
+ // Both links are added because they both contain vulnerable symbols
+ FixLinks: []string{"https://example.com/module/commit/1234", "https://example.com/module/commit/5678"},
}},
References: []*report.Reference{
{
