From 2fd6fa4e04cb5e97f25664d0c53686f45c0895cd Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Thu, 26 Oct 2023 18:28:08 -0300 Subject: [PATCH] Vulnerability processing should ignore software without version (#14612) #13615 - [X] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - ~[ ] Documented any API changes (docs/Using-Fleet/REST-API.md or docs/Contributing/API-for-contributors.md)~ - ~[ ] Documented any permissions changes (docs/Using Fleet/manage-access.md)~ - ~[ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements)~ - ~[ ] Added support on fleet's osquery simulator `cmd/osquery-perf` for new osquery data ingestion features.~ - [X] Added/updated tests - [X] Manual QA for all new/changed functionality - ~For Orbit and Fleet Desktop changes:~ - ~[ ] Manual QA must be performed in the three main OSs, macOS, Windows and Linux.~ - ~[ ] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)).~ --- changes/13615-ignore-software-without-version | 1 + docs/Using Fleet/Vulnerability-Processing.md | 2 + server/vulnerabilities/nvd/cpe.go | 29 +++++++++-- server/vulnerabilities/nvd/cpe_test.go | 48 ++++++++++++++++++- 4 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 changes/13615-ignore-software-without-version diff --git a/changes/13615-ignore-software-without-version b/changes/13615-ignore-software-without-version new file mode 100644 index 000000000..00bb70315 --- /dev/null +++ b/changes/13615-ignore-software-without-version @@ -0,0 +1 @@ +* Updated vulnerability processing to ignore software that has been ingested without a version. diff --git a/docs/Using Fleet/Vulnerability-Processing.md b/docs/Using Fleet/Vulnerability-Processing.md index 7c7d34c68..5f543dbd6 100644 --- a/docs/Using Fleet/Vulnerability-Processing.md +++ b/docs/Using Fleet/Vulnerability-Processing.md @@ -301,6 +301,8 @@ With a somewhat normalized list of software, in order to search CVEs for it, we As described briefly above, we do this by translating the NVD database of CPEs into a [sqlite database that helps Fleet do the lookup of CPEs very quickly](https://github.com/fleetdm/nvd). +NOTE: Software that was ingested with an empty `version` field will be ignored by the NVD vulnerability processing. + #### How accurate is this translation process? This is the most error prone part of the process. diff --git a/server/vulnerabilities/nvd/cpe.go b/server/vulnerabilities/nvd/cpe.go index 912c7c3fc..11bead28b 100644 --- a/server/vulnerabilities/nvd/cpe.go +++ b/server/vulnerabilities/nvd/cpe.go @@ -397,12 +397,33 @@ func TranslateSoftwareToCPE( if err != nil { return ctxerr.Wrap(ctx, err, "getting value from iterator") } - cpe, err := CPEFromSoftware(db, software, cpeTranslations, reCache) - if err != nil { - level.Error(logger).Log("software->cpe", "error translating to CPE, skipping...", "err", err) - continue + var cpe string + // Skip software without version to avoid false positives in the CPE + // matching process. + if software.Version == "" { + level.Debug(logger).Log( + "msg", "skipping software without version", + "software", software.Name, + "source", software.Source, + ) + // We want to continue here in case the software had an invalid CPE + // generated by a previous version of Fleet. + } else { + cpe, err = CPEFromSoftware(db, software, cpeTranslations, reCache) + if err != nil { + level.Error(logger).Log( + "msg", "error translating to CPE, skipping", + "software", software.Name, + "version", software.Version, + "source", software.Source, + "err", err, + ) + continue + } } if cpe == software.GenerateCPE { + // If the generated CPE hasn't changed from what's already stored in the DB + // then we don't need to do anything. continue } diff --git a/server/vulnerabilities/nvd/cpe_test.go b/server/vulnerabilities/nvd/cpe_test.go index 53520f5c6..1ad545b5a 100644 --- a/server/vulnerabilities/nvd/cpe_test.go +++ b/server/vulnerabilities/nvd/cpe_test.go @@ -297,8 +297,6 @@ func TestConsumeCPEBuffer(t *testing.T) { } func TestTranslateSoftwareToCPE(t *testing.T) { - nettest.Run(t) - tempDir := t.TempDir() ds := new(mock.Store) @@ -361,6 +359,52 @@ func TestTranslateSoftwareToCPE(t *testing.T) { assert.True(t, iterator.closed) } +// TestTranslateSoftwareToCPEIgnoreEmptyVersion tests that TranslateSoftwareToCPE ignores +// software that was ingested with an empty version field. The test will simulate a previous +// version of Fleet storing an incorrect CPE for the software, to test that an upgrade +// will clear out the invalid CPE from the DB. +func TestTranslateSoftwareToCPEIgnoreEmptyVersion(t *testing.T) { + tempDir := t.TempDir() + + ds := new(mock.Store) + + // The incorrect CPE for the software should now be deleted because the ingested software doesn't + // have a version field. + ds.DeleteSoftwareCPEsFunc = func(ctx context.Context, cpes []fleet.SoftwareCPE) (int64, error) { + require.Len(t, cpes, 1) + require.Equal(t, cpes[0].SoftwareID, uint(1)) + return 1, nil + } + + ds.AllSoftwareIteratorFunc = func(ctx context.Context, q fleet.SoftwareIterQueryOptions) (fleet.SoftwareIterator, error) { + return &fakeSoftwareIterator{ + softwares: []*fleet.Software{ + { + ID: 1, + Name: "foobar", + Version: "", + BundleIdentifier: "vendor2", + Source: "apps", + // Set an incorrect CPE on the DB to simulate a CPE being generated incorrectly + // for this software on a previous version of Fleet. + GenerateCPE: "cpe:2.3:a:vendor2:foobar:*:*:*:*:*:macos:*:*", + }, + }, + }, nil + } + + items, err := cpedict.Decode(strings.NewReader(XmlCPETestDict)) + require.NoError(t, err) + + dbPath := filepath.Join(tempDir, "cpe.sqlite") + err = GenerateCPEDB(dbPath, items) + require.NoError(t, err) + + err = TranslateSoftwareToCPE(context.Background(), ds, tempDir, kitlog.NewNopLogger()) + require.NoError(t, err) + require.True(t, ds.DeleteSoftwareCPEsFuncInvoked) +} + func TestSyncsCPEFromURL(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { zw := gzip.NewWriter(w)