Skip to content

Commit

Permalink
feat: group DSA and its CVEs together (#1262)
Browse files Browse the repository at this point in the history
For container scanning, we should only display DSA results and hide
their related CVEs.
- Add DSA's related CVEs to the same group.
- Sort the group IDs with the DSA first. (DSA is more important than
CVE, and one DSA contains multiple CVEs)
- Don't print CVE records when there is a DSA (this reduces noise in
container scanning results).
  • Loading branch information
hogo6002 authored Sep 23, 2024
1 parent 56e3a99 commit 75a5eab
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 173 deletions.
228 changes: 76 additions & 152 deletions cmd/osv-scanner/__snapshots__/main_test.snap

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package output
package identifiers

import (
"strings"
)

func prefixOrder(prefix string) int {
if prefix == "CVE" {
// Highest precedence
if prefix == "DSA" {
// Special case: For container scanning, DSA contains multiple CVEs and is more accurate.
return 3
} else if prefix == "CVE" {
// Highest precedence for normal cases
return 2
} else if prefix == "GHSA" {
// Lowest precedence
Expand All @@ -26,16 +29,6 @@ func prefixOrderForDescription(prefix string) int {
return 2
}

// idSortFunc sorts IDs ascending by CVE < [ECO-SPECIFIC] < GHSA
func idSortFunc(a, b string) int {
return idSort(a, b, prefixOrder)
}

// idSortFuncForDescription sorts ID ascending by [ECO-SPECIFIC] < GHSA < CVE
func idSortFuncForDescription(a, b string) int {
return idSort(a, b, prefixOrderForDescription)
}

func idSort(a, b string, prefixOrd func(string) int) int {
prefixAOrd := prefixOrd(strings.Split(a, "-")[0])
prefixBOrd := prefixOrd(strings.Split(b, "-")[0])
Expand All @@ -48,3 +41,13 @@ func idSort(a, b string, prefixOrd func(string) int) int {

return strings.Compare(a, b)
}

// IDSortFunc sorts IDs ascending by CVE < [ECO-SPECIFIC] < GHSA
func IDSortFunc(a, b string) int {
return idSort(a, b, prefixOrder)
}

// IDSortFuncForDescription sorts ID ascending by [ECO-SPECIFIC] < GHSA < CVE
func IDSortFuncForDescription(a, b string) int {
return idSort(a, b, prefixOrderForDescription)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package output
package identifiers

import (
"slices"
Expand Down Expand Up @@ -36,7 +36,7 @@ func Test_idSortFunc(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

if got := idSortFunc(tt.args.a, tt.args.b); got != tt.want {
if got := IDSortFunc(tt.args.a, tt.args.b); got != tt.want {
t.Errorf("idSortFunc() = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -65,12 +65,19 @@ func Test_idSortFuncUsage(t *testing.T) {
},
want: "RUSTSEC-2012-1234",
},
{
args: []string{
"CVE-2012-1234",
"DSA-2012-1234",
},
want: "DSA-2012-1234",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

if got := slices.MinFunc(tt.args, idSortFunc); got != tt.want {
if got := slices.MinFunc(tt.args, IDSortFunc); got != tt.want {
t.Errorf("slices.MinFunc = %v, want %v", got, tt.want)
}
})
Expand Down
3 changes: 2 additions & 1 deletion internal/output/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"slices"
"strings"

"github.com/google/osv-scanner/internal/identifiers"
"github.com/google/osv-scanner/pkg/models"
"golang.org/x/exp/maps"
)
Expand Down Expand Up @@ -159,7 +160,7 @@ func mapIDsToGroupedSARIFFinding(vulns *models.VulnerabilityResults) map[string]
}

for _, gs := range results {
slices.SortFunc(gs.AliasedIDList, idSortFunc)
slices.SortFunc(gs.AliasedIDList, identifiers.IDSortFunc)
gs.AliasedIDList = slices.Compact(gs.AliasedIDList)
gs.DisplayID = gs.AliasedIDList[0]
}
Expand Down
5 changes: 3 additions & 2 deletions internal/output/sarif.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"text/template"

"github.com/google/osv-scanner/internal/identifiers"
"github.com/google/osv-scanner/internal/url"
"github.com/google/osv-scanner/internal/utility/results"
"github.com/google/osv-scanner/internal/version"
Expand Down Expand Up @@ -191,7 +192,7 @@ func createSARIFHelpText(gv *groupedSARIFFinding) string {
Details: strings.ReplaceAll(v.Details, "\n", "\n> "),
})
}
slices.SortFunc(vulnDescriptions, func(a, b VulnDescription) int { return idSortFunc(a.ID, b.ID) })
slices.SortFunc(vulnDescriptions, func(a, b VulnDescription) int { return identifiers.IDSortFunc(a.ID, b.ID) })

helpText := strings.Builder{}

Expand Down Expand Up @@ -250,7 +251,7 @@ func PrintSARIFReport(vulnResult *models.VulnerabilityResults, outputWriter io.W
// or use a random long description.
var shortDescription, longDescription string
ids := slices.Clone(gv.AliasedIDList)
slices.SortFunc(ids, idSortFuncForDescription)
slices.SortFunc(ids, identifiers.IDSortFuncForDescription)

for _, id := range ids {
v := gv.AliasedVulns[id]
Expand Down
5 changes: 5 additions & 0 deletions internal/output/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ func tableBuilderInner(vulnResult *models.VulnerabilityResults, calledVulns bool

for _, vuln := range group.IDs {
links = append(links, OSVBaseVulnerabilityURL+text.Bold.Sprintf("%s", vuln))

// For container scanning results, if there is a DSA, then skip printing its sub-CVEs.
if strings.Split(vuln, "-")[0] == "DSA" {
break
}
}

outputRow = append(outputRow, strings.Join(links, "\n"))
Expand Down
3 changes: 2 additions & 1 deletion pkg/grouper/grouper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"golang.org/x/exp/maps"

"github.com/google/osv-scanner/internal/identifiers"
"github.com/google/osv-scanner/pkg/models"
)

Expand Down Expand Up @@ -56,7 +57,7 @@ func Group(vulns []IDAliases) []models.GroupInfo {
result := make([]models.GroupInfo, 0, len(sortedKeys))
for _, key := range sortedKeys {
// Sort the strings so they are always in the same order
sort.Strings(extractedGroups[key])
slices.SortFunc(extractedGroups[key], identifiers.IDSortFunc)

// Add IDs to aliases
extractedAliases[key] = append(extractedAliases[key], extractedGroups[key]...)
Expand Down
14 changes: 13 additions & 1 deletion pkg/grouper/grouper_models.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package grouper

import "github.com/google/osv-scanner/pkg/models"
import (
"strings"

"github.com/google/osv-scanner/pkg/models"
)

type IDAliases struct {
ID string
Expand All @@ -14,6 +18,14 @@ func ConvertVulnerabilityToIDAliases(c []models.Vulnerability) []IDAliases {
ID: v.ID,
Aliases: v.Aliases,
}

// For Debian Security Advisory data,
// all related CVEs should be bundled together, as they are part of this DSA.
// TODO(gongh@): Revisit and provide a universal way to handle all Linux distro advisories.
if strings.Split(v.ID, "-")[0] == "DSA" {
idAliases.Aliases = append(idAliases.Aliases, v.Related...)
}

output = append(output, idAliases)
}

Expand Down

0 comments on commit 75a5eab

Please sign in to comment.