Skip to content

Commit

Permalink
fix(redhat): correct rewriting of recommendations for the same vulner…
Browse files Browse the repository at this point in the history 
…ability (#8063)
  • Loading branch information
@DmitriyLewen
DmitriyLewen authored Dec 10, 2024
1 parent 156a2aa commit 4202c4b
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 54 deletions.
61 changes: 21 additions & 40 deletions pkg/detector/ospkg/redhat/redhat.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import (
"time"

version "github.com/knqyf263/go-rpm-version"
"github.com/samber/lo"
"golang.org/x/xerrors"

dbTypes "github.com/aquasecurity/trivy-db/pkg/types"
ustrings "github.com/aquasecurity/trivy-db/pkg/utils/strings"
redhat "github.com/aquasecurity/trivy-db/pkg/vulnsrc/redhat-oval"
"github.com/aquasecurity/trivy-db/pkg/vulnsrc/vulnerability"
osver "github.com/aquasecurity/trivy/pkg/detector/ospkg/version"
Expand Down Expand Up @@ -116,24 +114,35 @@ func (s *Scanner) detect(osVer string, pkg ftypes.Package) ([]types.DetectedVuln
return nil, xerrors.Errorf("failed to get Red Hat advisories: %w", err)
}

installed := utils.FormatVersion(pkg)
installedVersion := version.NewVersion(installed)

uniqVulns := make(map[string]types.DetectedVulnerability)
// Choose the latest fixed version for each CVE-ID (empty for unpatched vulns).
// Take the single RHSA-ID with the latest fixed version (for patched vulns).
uniqAdvisories := make(map[string]dbTypes.Advisory)
for _, adv := range advisories {
// if Arches for advisory is empty or pkg.Arch is "noarch", then any Arches are affected
// If Arches for advisory are empty or pkg.Arch is "noarch", then any Arches are affected
if len(adv.Arches) != 0 && pkg.Arch != "noarch" {
if !slices.Contains(adv.Arches, pkg.Arch) {
continue
}
}

vulnID := adv.VulnerabilityID
if a, ok := uniqAdvisories[adv.VulnerabilityID]; ok {
if version.NewVersion(a.FixedVersion).LessThan(version.NewVersion(adv.FixedVersion)) {
uniqAdvisories[adv.VulnerabilityID] = adv
}
} else {
uniqAdvisories[adv.VulnerabilityID] = adv
}
}

var vulns []types.DetectedVulnerability
for _, adv := range uniqAdvisories {
vuln := types.DetectedVulnerability{
VulnerabilityID: vulnID,
VulnerabilityID: adv.VulnerabilityID,
VendorIDs: adv.VendorIDs, // Will be empty for unpatched vulnerabilities
PkgID: pkg.ID,
PkgName: pkg.Name,
InstalledVersion: utils.FormatVersion(pkg),
FixedVersion: version.NewVersion(adv.FixedVersion).String(), // Will be empty for unpatched vulnerabilities
PkgIdentifier: pkg.Identifier,
Status: adv.Status,
Layer: pkg.Layer,
Expand All @@ -144,43 +153,15 @@ func (s *Scanner) detect(osVer string, pkg ftypes.Package) ([]types.DetectedVuln
Custom: adv.Custom,
}

// unpatched vulnerabilities
if adv.FixedVersion == "" {
// Red Hat may contain several advisories for the same vulnerability (RHSA advisories).
// To avoid overwriting the fixed version by mistake, we should skip unpatched vulnerabilities if they were added earlier
if _, ok := uniqVulns[vulnID]; !ok {
uniqVulns[vulnID] = vuln
}
continue
}

// patched vulnerabilities
fixedVersion := version.NewVersion(adv.FixedVersion)
if installedVersion.LessThan(fixedVersion) {
vuln.VendorIDs = adv.VendorIDs
vuln.FixedVersion = fixedVersion.String()

if v, ok := uniqVulns[vulnID]; ok {
// In case two advisories resolve the same CVE-ID.
// e.g. The first fix might be incomplete.
v.VendorIDs = ustrings.Unique(append(v.VendorIDs, vuln.VendorIDs...))

// The newer fixed version should be taken.
if version.NewVersion(v.FixedVersion).LessThan(fixedVersion) {
v.FixedVersion = vuln.FixedVersion
}
uniqVulns[vulnID] = v
} else {
uniqVulns[vulnID] = vuln
}
// Keep unpatched and affected vulnerabilities
if adv.FixedVersion == "" || version.NewVersion(vuln.InstalledVersion).LessThan(version.NewVersion(adv.FixedVersion)) {
vulns = append(vulns, vuln)
}
}

vulns := lo.Values(uniqVulns)
sort.Slice(vulns, func(i, j int) bool {
return vulns[i].VulnerabilityID < vulns[j].VulnerabilityID
})

return vulns, nil
}

Expand Down
84 changes: 71 additions & 13 deletions pkg/detector/ospkg/redhat/redhat_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ func TestScanner_Detect(t *testing.T) {
},
},
{
VulnerabilityID: "CVE-2019-12735",
VendorIDs: []string{"RHSA-2019:1619"},
VulnerabilityID: "CVE-2019-12735",
VendorIDs: []string{
"RHSA-2019:1619",
},
PkgName: "vim-minimal",
InstalledVersion: "2:7.4.160-5.el7",
FixedVersion: "2:7.4.160-6.el7_6",
Expand Down Expand Up @@ -124,8 +126,10 @@ func TestScanner_Detect(t *testing.T) {
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2019-17007",
VendorIDs: []string{"RHSA-2021:0876"},
VulnerabilityID: "CVE-2019-17007",
VendorIDs: []string{
"RHSA-2021:0876",
},
PkgName: "nss",
InstalledVersion: "3.36.0-7.1.el7_6",
FixedVersion: "3.36.0-9.el7_6",
Expand All @@ -141,7 +145,6 @@ func TestScanner_Detect(t *testing.T) {
VulnerabilityID: "CVE-2020-12403",
VendorIDs: []string{
"RHSA-2021:0538",
"RHSA-2021:0876",
},
PkgName: "nss",
InstalledVersion: "3.36.0-7.1.el7_6",
Expand All @@ -156,6 +159,53 @@ func TestScanner_Detect(t *testing.T) {
},
},
},
{
name: "happy path: CVE-ID and RHSA-ID for same vulnerability",
fixtures: []string{
"testdata/fixtures/redhat.yaml",
"testdata/fixtures/cpe.yaml",
},
args: args{
osVer: "8.3",
pkgs: []ftypes.Package{
{
Name: "expat",
Version: "2.2.5",
Release: "16.el8_10",
Epoch: 0,
Arch: "x86_64",
SrcName: "expat",
SrcVersion: "2.2.5",
SrcRelease: "16.el8_10",
SrcEpoch: 0,
Layer: ftypes.Layer{
DiffID: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
},
BuildInfo: &ftypes.BuildInfo{
ContentSets: []string{"rhel-8-for-x86_64-baseos-rpms"},
},
},
},
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2024-45490",
VendorIDs: []string{
"RHSA-2024:6989-3",
},
PkgName: "expat",
InstalledVersion: "2.2.5-16.el8_10",
FixedVersion: "2.2.5-18.el8_10",
SeveritySource: vulnerability.RedHat,
Vulnerability: dbTypes.Vulnerability{
Severity: dbTypes.SeverityMedium.String(),
},
Layer: ftypes.Layer{
DiffID: "sha256:932da51564135c98a49a34a193d6cd363d8fa4184d957fde16c9d8527b3f3b02",
},
},
},
},
{
name: "happy path: package without architecture",
fixtures: []string{
Expand Down Expand Up @@ -186,8 +236,10 @@ func TestScanner_Detect(t *testing.T) {
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2016-5195",
VendorIDs: []string{"RHSA-2017:0372"},
VulnerabilityID: "CVE-2016-5195",
VendorIDs: []string{
"RHSA-2017:0372",
},
PkgName: "kernel-headers",
InstalledVersion: "3.10.0-1127.19-1.el7",
FixedVersion: "4.5.0-15.2.1.el7",
Expand Down Expand Up @@ -231,8 +283,10 @@ func TestScanner_Detect(t *testing.T) {
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2016-5195",
VendorIDs: []string{"RHSA-2016:2098"},
VulnerabilityID: "CVE-2016-5195",
VendorIDs: []string{
"RHSA-2016:2098",
},
PkgName: "kernel-headers",
InstalledVersion: "3.10.0-326.36-3.el7",
FixedVersion: "3.10.0-327.36.3.el7",
Expand Down Expand Up @@ -266,8 +320,10 @@ func TestScanner_Detect(t *testing.T) {
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2019-12735",
VendorIDs: []string{"RHSA-2019:1619"},
VulnerabilityID: "CVE-2019-12735",
VendorIDs: []string{
"RHSA-2019:1619",
},
PkgName: "vim-minimal",
InstalledVersion: "2:7.4.160-5.el8",
FixedVersion: "2:7.4.160-7.el8_7",
Expand Down Expand Up @@ -308,8 +364,10 @@ func TestScanner_Detect(t *testing.T) {
},
want: []types.DetectedVulnerability{
{
VulnerabilityID: "CVE-2019-11043",
VendorIDs: []string{"RHSA-2020:0322"},
VulnerabilityID: "CVE-2019-11043",
VendorIDs: []string{
"RHSA-2020:0322",
},
PkgName: "php",
InstalledVersion: "7.2.10-1.module_el8.2.0+313+b04d0a66",
FixedVersion: "7.2.11-1.1.module+el8.0.0+4664+17bd8d65",
Expand Down
65 changes: 64 additions & 1 deletion pkg/detector/ospkg/redhat/testdata/fixtures/redhat.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,67 @@
- ID: CVE-2016-5195
Severity: 3
Arches:
- aarch64
- aarch64
- bucket: expat
pairs:
- key: RHSA-2024:6989-2 # created for test only
value:
Entries:
- FixedVersion: 0:2.2.5-17.el8_10
Affected:
- 4
Arches:
- x86_64
Cves:
- ID: CVE-2024-45490
Severity: 2
- key: RHSA-2024:6989-3 # created for test only
value:
Entries:
- FixedVersion: 0:2.2.5-18.el8_10
Affected:
- 4
Arches:
- x86_64
Cves:
- ID: CVE-2024-45490
Severity: 2
- key: CVE-2024-45490
value:
Entries:
- Affected:
- 4
Cves:
- Severity: 2
Status: 5
- key: CVE-2024-45491
value:
Entries:
- Affected:
- 4
Cves:
- Severity: 2
Status: 5
- key: RHSA-2024:6989
value:
Entries:
- FixedVersion: 0:2.2.5-15.el8_10
Affected:
- 4
Arches:
- x86_64
Cves:
- ID: CVE-2024-45490
Severity: 2
- ID: CVE-2024-45491
Severity: 2
- ID: CVE-2024-45492
Severity: 2
- key: CVE-2024-45492
value:
Entries:
- Affected:
- 4
Cves:
- Severity: 2
Status: 5

0 comments on commit 4202c4b

Please sign in to comment.