fix issue with duplicate vulns detected using nvd (#8613)

The OVAL analyzer falsely assumes that any vulnerabilities detected on a
host only come from OVAL. However, it is possible that NVD detects
vulnerabilities on these hosts even though it excludes software from
deb_packages and rpm_packages. For example, a python package twisted
v22.20 has a vulnerability CVE-2022-39348 detected by NVD. The OVAL
analyzer would delete this vulnerability, and it would be re-inserted by
the NVD scanner on the next run. This creates a loop.

The fix is to only delete vulnerabilities that are actually detected
using OVAL. We already store this in the source column in the
software_cve table.
This commit is contained in:
Michal Nicpon
2022-11-10 10:28:00 -07:00
committed by GitHub
parent ff969e8ddc
commit 9ad1721efd
9 changed files with 235 additions and 43 deletions
+2
View File
@@ -0,0 +1,2 @@
* Fixed a bug where duplicate vulnerability webhook requests, jira, and zendesk tickets were being made when scanning for vulnerabilities.
This affected ubuntu and redhat hosts that support OVAL vulnerability detection.
+184
View File
@@ -2,6 +2,7 @@ package main
import (
"context"
"encoding/json"
"errors"
"io/ioutil"
"net/http"
@@ -13,9 +14,11 @@ import (
"testing"
"time"
"github.com/fleetdm/fleet/v4/pkg/nettest"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mock"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/fleetdm/fleet/v4/server/service"
kitlog "github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
@@ -276,6 +279,187 @@ func TestCronVulnerabilitiesCreatesDatabasesPath(t *testing.T) {
}, 5*time.Minute, 30*time.Second)
}
type softwareIterator struct {
index int
softwares []*fleet.Software
}
func (f *softwareIterator) Next() bool {
return f.index < len(f.softwares)
}
func (f *softwareIterator) Value() (*fleet.Software, error) {
s := f.softwares[f.index]
f.index++
return s, nil
}
func (f *softwareIterator) Err() error { return nil }
func (f *softwareIterator) Close() error { return nil }
func TestScanVulnerabilities(t *testing.T) {
nettest.Run(t)
logger := kitlog.NewNopLogger()
logger = level.NewFilter(logger, level.AllowDebug())
ctx := context.Background()
webhookCount := 0
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
webhookCount++
var payload map[string]json.RawMessage
err := json.NewDecoder(r.Body).Decode(&payload)
require.NoError(t, err)
expected := `
{
"cve": "CVE-2022-39348",
"details_link": "https://nvd.nist.gov/vuln/detail/CVE-2022-39348",
"epss_probability": 0.0089,
"cvss_score": 5.4,
"cisa_known_exploit": false,
"hosts_affected": [
{
"id": 1,
"hostname": "1",
"display_name": "1",
"url": "hosts/1"
}
]
}
`
require.JSONEq(t, expected, string(payload["vulnerability"]))
}))
appConfig := &fleet.AppConfig{
Features: fleet.Features{
EnableSoftwareInventory: true,
},
WebhookSettings: fleet.WebhookSettings{
VulnerabilitiesWebhook: fleet.VulnerabilitiesWebhookSettings{
Enable: true,
DestinationURL: svr.URL,
},
},
}
ds := new(mock.Store)
ds.InsertCVEMetaFunc = func(ctx context.Context, x []fleet.CVEMeta) error {
return nil
}
ds.AllSoftwareWithoutCPEIteratorFunc = func(ctx context.Context, excludedPlatforms []string) (fleet.SoftwareIterator, error) {
iterator := &softwareIterator{
softwares: []*fleet.Software{
{
ID: 1,
Name: "Twisted",
Version: "22.2.0",
BundleIdentifier: "",
Source: "python_packages",
},
},
}
return iterator, nil
}
ds.ListSoftwareCPEsFunc = func(ctx context.Context) ([]fleet.SoftwareCPE, error) {
return []fleet.SoftwareCPE{
{
ID: 1,
SoftwareID: 1,
CPE: "cpe:2.3:a:twistedmatrix:twisted:22.2.0:*:*:*:*:python:*:*",
},
}, nil
}
ds.InsertSoftwareVulnerabilitiesFunc = func(ctx context.Context, vulns []fleet.SoftwareVulnerability, src fleet.VulnerabilitySource) (int64, error) {
return 1, nil
}
ds.AddCPEForSoftwareFunc = func(ctx context.Context, software fleet.Software, cpe string) error {
return nil
}
ds.OSVersionsFunc = func(ctx context.Context, teamID *uint, platform *string, name *string, version *string) (*fleet.OSVersions, error) {
return &fleet.OSVersions{
CountsUpdatedAt: time.Now(),
OSVersions: []fleet.OSVersion{
{HostsCount: 1, Name: "Ubuntu 22.04.1 LTS", Platform: "ubuntu"},
},
}, nil
}
ds.HostIDsByOSVersionFunc = func(ctx context.Context, osVersion fleet.OSVersion, offset int, limit int) ([]uint, error) {
if offset == 0 {
return []uint{1}, nil
}
return []uint{}, nil
}
ds.ListSoftwareForVulnDetectionFunc = func(ctx context.Context, hostID uint) ([]fleet.Software, error) {
return []fleet.Software{
{
ID: 1,
Name: "Twisted",
Version: "22.2.0",
BundleIdentifier: "",
Source: "python_packages",
},
}, nil
}
ds.ListSoftwareVulnerabilitiesByHostIDsSourceFunc = func(ctx context.Context, hostIDs []uint, source fleet.VulnerabilitySource) (map[uint][]fleet.SoftwareVulnerability, error) {
require.Equal(t, []uint{1}, hostIDs)
require.Equal(t, fleet.UbuntuOVALSource, source)
return map[uint][]fleet.SoftwareVulnerability{}, nil
}
ds.ListOperatingSystemsFunc = func(ctx context.Context) ([]fleet.OperatingSystem, error) {
return []fleet.OperatingSystem{
{
ID: 1,
Name: "Ubuntu",
Version: "22.04.1 LTS",
Arch: "x86_64",
KernelVersion: "5.10.124-linuxkit",
},
}, nil
}
ds.ListCVEsFunc = func(ctx context.Context, maxAge time.Duration) ([]fleet.CVEMeta, error) {
published := time.Date(2022, time.October, 26, 14, 15, 0, 0, time.UTC)
return []fleet.CVEMeta{
{
CVE: "CVE-2022-39348",
CVSSScore: ptr.Float64(5.4),
EPSSProbability: ptr.Float64(0.0089),
CISAKnownExploit: ptr.Bool(false),
Published: &published,
},
}, nil
}
ds.HostsBySoftwareIDsFunc = func(ctx context.Context, softwareIDs []uint) ([]*fleet.HostShort, error) {
return []*fleet.HostShort{
{
ID: 1,
Hostname: "1",
DisplayName: "1",
},
}, nil
}
vulnPath := t.TempDir()
config := config.VulnerabilitiesConfig{
DatabasesPath: vulnPath,
Periodicity: 10 * time.Second,
CurrentInstanceChecks: "auto",
}
err := scanVulnerabilities(ctx, ds, logger, &config, appConfig, vulnPath, &fleet.LicenseInfo{Tier: "premium"})
require.NoError(t, err)
// ensure that nvd vulnerabilities are not deleted
require.False(t, ds.DeleteSoftwareVulnerabilitiesFuncInvoked)
// ensure that webhook was called
require.Equal(t, 1, webhookCount)
}
func TestScanVulnerabilitiesMkdirFailsIfVulnPathIsFile(t *testing.T) {
logger := kitlog.NewNopLogger()
logger = level.NewFilter(logger, level.AllowDebug())
+13 -9
View File
@@ -1063,32 +1063,36 @@ func (ds *Datastore) InsertSoftwareVulnerabilities(
return count, nil
}
func (ds *Datastore) ListSoftwareVulnerabilities(
func (ds *Datastore) ListSoftwareVulnerabilitiesByHostIDsSource(
ctx context.Context,
hostIDs []uint,
source fleet.VulnerabilitySource,
) (map[uint][]fleet.SoftwareVulnerability, error) {
result := make(map[uint][]fleet.SoftwareVulnerability)
type softwareVulnerabilityWithHostId struct {
fleet.SoftwareVulnerability
HostId uint `db:"host_id"`
HostID uint `db:"host_id"`
}
var queryR []softwareVulnerabilityWithHostId
stmt := dialect.
From(goqu.T("software_cve").As("cve")).
From(goqu.T("software_cve").As("sc")).
Join(
goqu.T("host_software").As("hs"),
goqu.On(goqu.Ex{
"cve.software_id": goqu.I("hs.software_id"),
"sc.software_id": goqu.I("hs.software_id"),
}),
).
Select(
goqu.I("hs.host_id").As("host_id"),
goqu.I("cve.software_id"),
goqu.I("cve"),
goqu.I("hs.host_id"),
goqu.I("sc.software_id"),
goqu.I("sc.cve"),
).
Where(goqu.C("host_id").In(hostIDs))
Where(
goqu.I("hs.host_id").In(hostIDs),
goqu.I("sc.source").Eq(source),
)
sql, args, err := stmt.ToSQL()
if err != nil {
@@ -1100,7 +1104,7 @@ func (ds *Datastore) ListSoftwareVulnerabilities(
}
for _, r := range queryR {
result[r.HostId] = append(result[r.HostId], r.SoftwareVulnerability)
result[r.HostID] = append(result[r.HostID], r.SoftwareVulnerability)
}
return result, nil
+11 -11
View File
@@ -38,7 +38,7 @@ func TestSoftware(t *testing.T) {
{"HostsBySoftwareIDs", testHostsBySoftwareIDs},
{"UpdateHostSoftware", testUpdateHostSoftware},
{"ListSoftwareByHostIDShort", testListSoftwareByHostIDShort},
{"ListSoftwareVulnerabilities", testListSoftwareVulnerabilities},
{"ListSoftwareVulnerabilitiesByHostIDsSource", testListSoftwareVulnerabilitiesByHostIDsSource},
{"InsertSoftwareVulnerabilities", testInsertSoftwareVulnerabilities},
{"ListCVEs", testListCVEs},
{"ListSoftwareForVulnDetection", testListSoftwareForVulnDetection},
@@ -1424,7 +1424,7 @@ func testListSoftwareByHostIDShort(t *testing.T, ds *Datastore) {
require.Len(t, software, 0)
}
func testListSoftwareVulnerabilities(t *testing.T, ds *Datastore) {
func testListSoftwareVulnerabilitiesByHostIDsSource(t *testing.T, ds *Datastore) {
ctx := context.Background()
host := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
@@ -1459,17 +1459,17 @@ func testListSoftwareVulnerabilities(t *testing.T, ds *Datastore) {
}
n, err := ds.InsertSoftwareVulnerabilities(ctx, vulns, fleet.NVDSource)
require.NoError(t, err)
require.Equal(t, int(n), 2)
require.Equal(t, int64(2), n)
expectedCVEs := []string{"cve-123", "cve-456"}
result, err := ds.ListSoftwareVulnerabilitiesByHostIDsSource(ctx, []uint{host.ID}, fleet.NVDSource)
require.NoError(t, err)
actualCVEs := make([]string, 0)
result, err := ds.ListSoftwareVulnerabilities(ctx, []uint{host.ID})
var actualCVEs []string
for _, r := range result[host.ID] {
actualCVEs = append(actualCVEs, r.CVE)
}
require.NoError(t, err)
expectedCVEs := []string{"cve-123", "cve-456"}
require.ElementsMatch(t, expectedCVEs, actualCVEs)
for _, r := range result[host.ID] {
@@ -1482,8 +1482,8 @@ func testInsertSoftwareVulnerabilities(t *testing.T, ds *Datastore) {
t.Run("no vulnerabilities to insert", func(t *testing.T) {
r, err := ds.InsertSoftwareVulnerabilities(ctx, nil, fleet.UbuntuOVALSource)
require.Zero(t, r)
require.NoError(t, err)
require.Zero(t, r)
})
t.Run("duplicated vulnerabilities", func(t *testing.T) {
@@ -1508,9 +1508,9 @@ func testInsertSoftwareVulnerabilities(t *testing.T, ds *Datastore) {
n, err := ds.InsertSoftwareVulnerabilities(ctx, vulns, fleet.UbuntuOVALSource)
require.NoError(t, err)
require.Equal(t, 1, int(n))
require.Equal(t, int64(1), n)
storedVulns, err := ds.ListSoftwareVulnerabilities(ctx, []uint{host.ID})
storedVulns, err := ds.ListSoftwareVulnerabilitiesByHostIDsSource(ctx, []uint{host.ID}, fleet.UbuntuOVALSource)
require.NoError(t, err)
occurrence := make(map[string]int)
@@ -1546,7 +1546,7 @@ func testInsertSoftwareVulnerabilities(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Equal(t, 0, int(n))
storedVulns, err := ds.ListSoftwareVulnerabilities(ctx, []uint{host.ID})
storedVulns, err := ds.ListSoftwareVulnerabilitiesByHostIDsSource(ctx, []uint{host.ID}, fleet.UbuntuOVALSource)
require.NoError(t, err)
occurrence := make(map[string]int)
+2 -1
View File
@@ -386,10 +386,11 @@ type Datastore interface {
///////////////////////////////////////////////////////////////////////////////
// SoftwareStore
// ListSoftwareForVulnDetection returns all software for the given hostID with only the fields
// used for vulnerability detection populated (id, name, version, cpe_id, cpe)
ListSoftwareForVulnDetection(ctx context.Context, hostID uint) ([]Software, error)
ListSoftwareVulnerabilities(ctx context.Context, hostIDs []uint) (map[uint][]SoftwareVulnerability, error)
ListSoftwareVulnerabilitiesByHostIDsSource(ctx context.Context, hostIDs []uint, source VulnerabilitySource) (map[uint][]SoftwareVulnerability, error)
LoadHostSoftware(ctx context.Context, host *Host, includeCVEScores bool) error
AllSoftwareWithoutCPEIterator(ctx context.Context, excludedPlatforms []string) (SoftwareIterator, error)
AddCPEForSoftware(ctx context.Context, software Software, cpe string) error
+6 -6
View File
@@ -303,7 +303,7 @@ type DeleteIntegrationsFromTeamsFunc func(ctx context.Context, deletedIntgs flee
type ListSoftwareForVulnDetectionFunc func(ctx context.Context, hostID uint) ([]fleet.Software, error)
type ListSoftwareVulnerabilitiesFunc func(ctx context.Context, hostIDs []uint) (map[uint][]fleet.SoftwareVulnerability, error)
type ListSoftwareVulnerabilitiesByHostIDsSourceFunc func(ctx context.Context, hostIDs []uint, source fleet.VulnerabilitySource) (map[uint][]fleet.SoftwareVulnerability, error)
type LoadHostSoftwareFunc func(ctx context.Context, host *fleet.Host, includeCVEScores bool) error
@@ -935,8 +935,8 @@ type DataStore struct {
ListSoftwareForVulnDetectionFunc ListSoftwareForVulnDetectionFunc
ListSoftwareForVulnDetectionFuncInvoked bool
ListSoftwareVulnerabilitiesFunc ListSoftwareVulnerabilitiesFunc
ListSoftwareVulnerabilitiesFuncInvoked bool
ListSoftwareVulnerabilitiesByHostIDsSourceFunc ListSoftwareVulnerabilitiesByHostIDsSourceFunc
ListSoftwareVulnerabilitiesByHostIDsSourceFuncInvoked bool
LoadHostSoftwareFunc LoadHostSoftwareFunc
LoadHostSoftwareFuncInvoked bool
@@ -1955,9 +1955,9 @@ func (s *DataStore) ListSoftwareForVulnDetection(ctx context.Context, hostID uin
return s.ListSoftwareForVulnDetectionFunc(ctx, hostID)
}
func (s *DataStore) ListSoftwareVulnerabilities(ctx context.Context, hostIDs []uint) (map[uint][]fleet.SoftwareVulnerability, error) {
s.ListSoftwareVulnerabilitiesFuncInvoked = true
return s.ListSoftwareVulnerabilitiesFunc(ctx, hostIDs)
func (s *DataStore) ListSoftwareVulnerabilitiesByHostIDsSource(ctx context.Context, hostIDs []uint, source fleet.VulnerabilitySource) (map[uint][]fleet.SoftwareVulnerability, error) {
s.ListSoftwareVulnerabilitiesByHostIDsSourceFuncInvoked = true
return s.ListSoftwareVulnerabilitiesByHostIDsSourceFunc(ctx, hostIDs, source)
}
func (s *DataStore) LoadHostSoftware(ctx context.Context, host *fleet.Host, includeCVEScores bool) error {
+9 -10
View File
@@ -50,20 +50,19 @@ func Analyze(
var offset int
for {
hIds, err := ds.HostIDsByOSVersion(ctx, ver, offset, hostsBatchSize)
offset += hostsBatchSize
hostIDs, err := ds.HostIDsByOSVersion(ctx, ver, offset, hostsBatchSize)
if err != nil {
return nil, err
}
if len(hIds) == 0 {
if len(hostIDs) == 0 {
break
}
offset += hostsBatchSize
foundInBatch := make(map[uint][]fleet.SoftwareVulnerability)
for _, hId := range hIds {
software, err := ds.ListSoftwareForVulnDetection(ctx, hId)
for _, hostID := range hostIDs {
software, err := ds.ListSoftwareForVulnDetection(ctx, hostID)
if err != nil {
return nil, err
}
@@ -72,16 +71,16 @@ func Analyze(
if err != nil {
return nil, err
}
foundInBatch[hId] = evalR
foundInBatch[hostID] = evalR
}
existingInBatch, err := ds.ListSoftwareVulnerabilities(ctx, hIds)
existingInBatch, err := ds.ListSoftwareVulnerabilitiesByHostIDsSource(ctx, hostIDs, source)
if err != nil {
return nil, err
}
for _, hId := range hIds {
insrt, del := utils.VulnsDelta(foundInBatch[hId], existingInBatch[hId])
for _, hostID := range hostIDs {
insrt, del := utils.VulnsDelta(foundInBatch[hostID], existingInBatch[hostID])
for _, i := range insrt {
toInsertSet[i.Key()] = i
}
+6 -6
View File
@@ -37,8 +37,7 @@ func extract(src, dst string, t require.TestingT) {
defer dstF.Close()
r := bzip2.NewReader(srcF)
// ignoring "G110: Potential DoS vulnerability via decompression bomb", as this is test code.
_, err = io.Copy(dstF, r) //nolint:gosec
_, err = io.Copy(dstF, r) //nolint:gosec // ignoring "G110: Potential DoS vulnerability via decompression bomb", as this is test code.
require.NoError(t, err)
}
@@ -143,11 +142,12 @@ func withTestFixutre(
}
func assertVulns(
t require.TestingT,
ds *mysql.Datastore,
vulnPath string,
h *fleet.Host,
p Platform,
t require.TestingT,
source fleet.VulnerabilitySource,
) {
ctx := context.Background()
@@ -180,7 +180,7 @@ func assertVulns(
}
require.NotEmpty(t, expected)
storedVulns, err := ds.ListSoftwareVulnerabilities(ctx, []uint{h.ID})
storedVulns, err := ds.ListSoftwareVulnerabilitiesByHostIDsSource(ctx, []uint{h.ID}, source)
require.NoError(t, err)
uniq := make(map[string]bool)
@@ -325,7 +325,7 @@ func TestOvalAnalyzer(t *testing.T) {
_, err := Analyze(ctx, ds, s.version, vulnPath, true)
require.NoError(t, err)
p := NewPlatform(s.version.Platform, s.version.Name)
assertVulns(ds, vulnPath, h, p, t)
assertVulns(t, ds, vulnPath, h, p, fleet.RHELOVALSource)
}, t)
}
})
@@ -358,7 +358,7 @@ func TestOvalAnalyzer(t *testing.T) {
require.NoError(t, err)
p := NewPlatform(v.Platform, v.Name)
assertVulns(ds, vulnPath, h, p, t)
assertVulns(t, ds, vulnPath, h, p, fleet.UbuntuOVALSource)
}, t)
}
})
+2
View File
@@ -93,12 +93,14 @@ func VulnsDelta[T fleet.Vulnerability](
for _, e := range existing {
if _, ok := foundSet[e.Key()]; !ok {
// existing not in found, delete
toDelete = append(toDelete, e)
}
}
for _, f := range found {
if _, ok := existingSet[f.Key()]; !ok {
// found not in existing, insert
toInsert = append(toInsert, f)
}
}