diff --git a/changes/11927-vuln-false-positive-icloud b/changes/11927-vuln-false-positive-icloud new file mode 100644 index 0000000000..0ca4fcfd6b --- /dev/null +++ b/changes/11927-vuln-false-positive-icloud @@ -0,0 +1,3 @@ +- Added CPEMatchingRule type, used for correcting false positives caused by incorrect entries in the + NVD dataset. +- Fixed false positives for iCloud on macOS. diff --git a/server/vulnerabilities/macoffice/analyzer.go b/server/vulnerabilities/macoffice/analyzer.go index 985fab8440..32d1dd0d68 100644 --- a/server/vulnerabilities/macoffice/analyzer.go +++ b/server/vulnerabilities/macoffice/analyzer.go @@ -49,7 +49,7 @@ func getLatestReleaseNotes(vulnPath string) (ReleaseNotes, error) { return relNotes, nil } -// collectVulnerabilities compares 'software' againts all 'release notes' returning all detected +// collectVulnerabilities compares 'software' against all 'release notes' returning all detected // vulnerabilities. func collectVulnerabilities( software *fleet.Software, diff --git a/server/vulnerabilities/macoffice/release_note.go b/server/vulnerabilities/macoffice/release_note.go index 0a646bad17..64c26351e8 100644 --- a/server/vulnerabilities/macoffice/release_note.go +++ b/server/vulnerabilities/macoffice/release_note.go @@ -49,7 +49,7 @@ func (or *ReleaseNote) Valid() bool { return len(or.Version) != 0 && len(or.SecurityUpdates) != 0 } -// CmpVersion compares the release note version againts 'otherVer' returning: +// CmpVersion compares the release note version against 'otherVer' returning: // -1 if rel. note version < other version // 0 if rel. note version == other version // 1 if rel. note version > other version diff --git a/server/vulnerabilities/nvd/cpe_matching_rule.go b/server/vulnerabilities/nvd/cpe_matching_rule.go new file mode 100644 index 0000000000..8d30bcb1a1 --- /dev/null +++ b/server/vulnerabilities/nvd/cpe_matching_rule.go @@ -0,0 +1,108 @@ +package nvd + +import ( + "errors" + "fmt" + "strings" + + "github.com/Masterminds/semver" + "github.com/facebookincubator/nvdtools/wfn" +) + +// CPEMatchingRuleSpec allows you to match against a CPE. Version ranges are supported via SemVer constraints. +type CPEMatchingRuleSpec struct { + Vendor string // Software vendor. + Product string // Software product name. + TargetSW string // Target software, this usually corresponds to the target OS. + + // Specifies a version constraint. See https://pkg.go.dev/github.com/Masterminds/semver@v1.5.0#hdr-Checking_Version_Constraints + // for reference. + SemVerConstraint string +} + +func (rule CPEMatchingRuleSpec) getCPEMeta() *wfn.Attributes { + return &wfn.Attributes{ + Vendor: rule.Vendor, + Product: rule.Product, + TargetSW: rule.TargetSW, + } +} + +// CPEMatchingRule allows you to express a matching rule based on some CPE properties, one or more +// CVEs and one or more SemVer constraint. This is used to 'fix' false positives resulting from bad +// data in the NVD dataset itself. +// For example: https://nvd.nist.gov/vuln/detail/CVE-2017-13797, one of the CPE entries specified is +// cpe:2.3:a:apple:icloud:*:*:*:*:*:*:*:* which will match with any iCloud installation, but the +// vulnerability in question only affects iCloud on Windows up to 7.0.x. +type CPEMatchingRule struct { + CPESpecs []CPEMatchingRuleSpec + // Set of CVEs that this rule targets + CVEs map[string]struct{} +} + +// CPEMatches returns true if both the provided CPE match the rule. +func (rule CPEMatchingRule) CPEMatches(cpeMeta *wfn.Attributes) bool { + if cpeMeta == nil { + return false + } + + ver, err := semver.NewVersion(wfn.StripSlashes(cpeMeta.Version)) + if err != nil { + return false + } + + for _, spec := range rule.CPESpecs { + // The SemVer constraint is validated at instantiation time, so it should be ok to ignore the error. + constraint, _ := semver.NewConstraint(spec.SemVerConstraint) + if cpeMeta.MatchWithoutVersion(spec.getCPEMeta()) && constraint.Check(ver) { + return true + } + } + + return false +} + +// Validate validates the rule, returns an error if there's something wrong. +func (rule CPEMatchingRule) Validate() error { + validateCPEPart := func(errPrefix, val string) error { + switch strings.TrimSpace(val) { + case "": + return fmt.Errorf("%s can't be empty", errPrefix) + case "*": + return fmt.Errorf("%s can't be 'ANY'", errPrefix) + case "-": + return fmt.Errorf("%s can't be 'NA'", errPrefix) + default: + return nil + } + } + + for _, spec := range rule.CPESpecs { + // Validate CPE parts + if err := validateCPEPart("Vendor", spec.Vendor); err != nil { + return err + } + if err := validateCPEPart("Product", spec.Product); err != nil { + return err + } + if err := validateCPEPart("TargetSW", spec.TargetSW); err != nil { + return err + } + // Validate SemVerConstraint + if _, err := semver.NewConstraint(spec.SemVerConstraint); err != nil { + return err + } + } + + // Validate CVEs entries + if len(rule.CVEs) == 0 { + return errors.New("At least one CVE is required") + } + for cve := range rule.CVEs { + if strings.TrimSpace(cve) == "" { + return errors.New("CVE can't be empty") + } + } + + return nil +} diff --git a/server/vulnerabilities/nvd/cpe_matching_rule_test.go b/server/vulnerabilities/nvd/cpe_matching_rule_test.go new file mode 100644 index 0000000000..f9120f8950 --- /dev/null +++ b/server/vulnerabilities/nvd/cpe_matching_rule_test.go @@ -0,0 +1,244 @@ +package nvd + +import ( + "errors" + "testing" + + "github.com/facebookincubator/nvdtools/wfn" + "github.com/stretchr/testify/require" +) + +func TestCPEProcessingRule(t *testing.T) { + buildRule := func() CPEMatchingRule { + return CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + + CVEs: map[string]struct{}{"CVE-123": {}}, + } + } + + buildCPEMeta := func() *wfn.Attributes { + cpeMeta, err := wfn.Parse("cpe:2.3:a:microsoft:word:1.2.3:*:*:*:*:windows:*:*") + require.NoError(t, err) + return cpeMeta + } + + t.Run("getCPEAttrs", func(t *testing.T) { + rule := buildRule() + result := rule.CPESpecs[0].getCPEMeta() + require.NotNil(t, result) + + expected := wfn.Attributes{ + Vendor: rule.CPESpecs[0].Vendor, + Product: rule.CPESpecs[0].Product, + TargetSW: rule.CPESpecs[0].TargetSW, + } + require.True(t, expected.MatchWithoutVersion(result)) + }) + + t.Run("Matches", func(t *testing.T) { + t.Run("is a match", func(t *testing.T) { + rule := buildRule() + cpeMeta := buildCPEMeta() + require.True(t, rule.CPEMatches(cpeMeta)) + }) + + t.Run("CPEMeta info is null", func(t *testing.T) { + rule := buildRule() + require.False(t, rule.CPEMatches(nil)) + }) + + t.Run("CPEs don't match", func(t *testing.T) { + rule := buildRule() + rule.CPESpecs[0].Vendor = "AMD" + cpeMeta := buildCPEMeta() + require.False(t, rule.CPEMatches(cpeMeta)) + }) + + t.Run("SemVer is a range constraint", func(t *testing.T) { + rule := buildRule() + rule.CPESpecs[0].SemVerConstraint = "1.0.0 - 2.0.0" + cpeMeta := buildCPEMeta() + require.True(t, rule.CPEMatches(cpeMeta)) + }) + }) + + t.Run("Validate", func(t *testing.T) { + testCases := []struct { + rule CPEMatchingRule + err error + }{ + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Vendor can't be empty"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "*", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Vendor can't be 'ANY'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "-", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Vendor can't be 'NA'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Product can't be empty"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "*", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Product can't be 'ANY'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "-", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("Product can't be 'NA'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("TargetSW can't be empty"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "*", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("TargetSW can't be 'ANY'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "-", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("TargetSW can't be 'NA'"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "windows", + SemVerConstraint: ".as.-as", + }, + }, + CVEs: map[string]struct{}{"CVE-123": {}}, + }, err: errors.New("improper constraint: .as.-as"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + }, err: errors.New("At least one CVE is required"), + }, + { + rule: CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "microsoft", + Product: "word", + TargetSW: "windows", + SemVerConstraint: "1.2.3", + }, + }, + CVEs: map[string]struct{}{"": {}, " ": {}, "CVE-123": {}}, + }, err: errors.New("CVE can't be empty"), + }, + } + + for _, tc := range testCases { + result := tc.rule.Validate() + require.Equal(t, tc.err, result) + } + }) +} diff --git a/server/vulnerabilities/nvd/cpe_matching_rules.go b/server/vulnerabilities/nvd/cpe_matching_rules.go new file mode 100644 index 0000000000..fd863a7863 --- /dev/null +++ b/server/vulnerabilities/nvd/cpe_matching_rules.go @@ -0,0 +1,128 @@ +package nvd + +import ( + "fmt" +) + +type CPEMatchingRules []CPEMatchingRule + +// GetKnownNVDBugRules returns a list of CPEMatchingRules used for +// ignoring false positives detected during the NVD vuln. detection process. +func GetKnownNVDBugRules() (CPEMatchingRules, error) { + rules := CPEMatchingRules{ + CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "apple", + Product: "icloud", + TargetSW: "windows", + SemVerConstraint: "< 7.1", + }, + }, + + CVEs: map[string]struct{}{ + "CVE-2017-13797": {}, + }, + }, + CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "apple", + Product: "icloud", + TargetSW: "windows", + SemVerConstraint: "<= 6.1.1", + }, + }, + + CVEs: map[string]struct{}{ + "CVE-2016-4613": {}, + "CVE-2017-2383": {}, + }, + }, + CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "apple", + Product: "icloud", + TargetSW: "windows", + SemVerConstraint: "<= 6.1.0", + }, + }, + + CVEs: map[string]struct{}{ + "CVE-2017-2366": {}, + }, + }, + CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "apple", + Product: "icloud", + TargetSW: "windows", + SemVerConstraint: "<= 6.0.0", + }, + }, + + CVEs: map[string]struct{}{ + "CVE-2016-4613": {}, + "CVE-2016-7583": {}, + }, + }, + CPEMatchingRule{ + CPESpecs: []CPEMatchingRuleSpec{ + { + Vendor: "apple", + Product: "icloud", + TargetSW: "windows", + SemVerConstraint: "<= 6.0.1", + }, + }, + + CVEs: map[string]struct{}{ + "CVE-2016-4692": {}, + "CVE-2016-4743": {}, + "CVE-2016-7578": {}, + "CVE-2016-7586": {}, + "CVE-2016-7587": {}, + "CVE-2016-7589": {}, + "CVE-2016-7592": {}, + "CVE-2016-7598": {}, + "CVE-2016-7599": {}, + "CVE-2016-7610": {}, + "CVE-2016-7611": {}, + "CVE-2016-7614": {}, + "CVE-2016-7632": {}, + "CVE-2016-7635": {}, + "CVE-2016-7639": {}, + "CVE-2016-7640": {}, + "CVE-2016-7641": {}, + "CVE-2016-7642": {}, + "CVE-2016-7645": {}, + "CVE-2016-7646": {}, + "CVE-2016-7648": {}, + "CVE-2016-7649": {}, + "CVE-2016-7652": {}, + "CVE-2016-7654": {}, + "CVE-2016-7656": {}, + }, + }, + } + + for i, rule := range rules { + if err := rule.Validate(); err != nil { + return nil, fmt.Errorf("invalid rule %d: %w", i, err) + } + } + + return rules, nil +} + +// FindMatch returns the first matching rule +func (rules CPEMatchingRules) FindMatch(cve string) (*CPEMatchingRule, bool) { + for _, rule := range rules { + if _, ok := rule.CVEs[cve]; ok { + return &rule, true + } + } + return nil, false +} diff --git a/server/vulnerabilities/nvd/cve.go b/server/vulnerabilities/nvd/cve.go index 92c01d4817..050744c36c 100644 --- a/server/vulnerabilities/nvd/cve.go +++ b/server/vulnerabilities/nvd/cve.go @@ -132,11 +132,25 @@ func TranslateCPEToCVE( return nil, nil } + knownNVDBugRules, err := GetKnownNVDBugRules() + if err != nil { + return nil, err + } + // we are using a map here to remove any duplicates - a vulnerability can be present in more than one // NVD feed file. vulns := make(map[string]fleet.SoftwareVulnerability) for _, file := range files { - foundVulns, err := checkCVEs(ctx, ds, logger, parsed, file, collectVulns) + + foundVulns, err := checkCVEs( + ctx, + ds, + logger, + parsed, + file, + collectVulns, + knownNVDBugRules, + ) if err != nil { return nil, err } @@ -179,6 +193,7 @@ func checkCVEs( softwareCPEs []softwareCPEWithNVDMeta, file string, collectVulns bool, + knownNVDBugRules CPEMatchingRules, ) ([]fleet.SoftwareVulnerability, error) { dict, err := cvefeed.LoadJSONDictionary(file) if err != nil { @@ -218,6 +233,14 @@ func checkCVEs( continue } + if rule, ok := knownNVDBugRules.FindMatch( + matches.CVE.ID(), + ); ok { + if !rule.CPEMatches(softwareCPE.meta) { + continue + } + } + vuln := fleet.SoftwareVulnerability{ SoftwareID: softwareCPE.SoftwareID, CVE: matches.CVE.ID(), diff --git a/server/vulnerabilities/nvd/cve_test.go b/server/vulnerabilities/nvd/cve_test.go index 8069f9cac0..05baec7c7f 100644 --- a/server/vulnerabilities/nvd/cve_test.go +++ b/server/vulnerabilities/nvd/cve_test.go @@ -109,25 +109,26 @@ var firefox93WindowsVulnerabilities = []string{ } var cvetests = []struct { - cpe string - cves []string + cpe string + excludedCVEs []string + includedCVEs []string // continuesToUpdate indicates if the product/software // continues to register new CVE vulnerabilities. continuestoUpdate bool }{ { cpe: "cpe:2.3:a:1password:1password:3.9.9:*:*:*:*:macos:*:*", - cves: []string{"CVE-2012-6369"}, + includedCVEs: []string{"CVE-2012-6369"}, continuestoUpdate: false, }, { cpe: "cpe:2.3:a:1password:1password:3.9.9:*:*:*:*:*:*:*", - cves: []string{"CVE-2012-6369"}, + includedCVEs: []string{"CVE-2012-6369"}, continuestoUpdate: false, }, { cpe: "cpe:2.3:a:pypa:pip:9.0.3:*:*:*:*:python:*:*", - cves: []string{ + includedCVEs: []string{ "CVE-2019-20916", "CVE-2021-3572", }, @@ -135,12 +136,49 @@ var cvetests = []struct { }, { cpe: "cpe:2.3:a:mozilla:firefox:93.0:*:*:*:*:windows:*:*", - cves: firefox93WindowsVulnerabilities, + includedCVEs: firefox93WindowsVulnerabilities, continuestoUpdate: true, }, { cpe: "cpe:2.3:a:mozilla:firefox:93.0.100:*:*:*:*:windows:*:*", - cves: firefox93WindowsVulnerabilities, + includedCVEs: firefox93WindowsVulnerabilities, + continuestoUpdate: true, + }, + { + cpe: "cpe:2.3:a:apple:icloud:1.0:*:*:*:*:macos:*:*", + excludedCVEs: []string{ + "CVE-2017-13797", + "CVE-2017-2383", + "CVE-2017-2366", + "CVE-2016-4613", + "CVE-2016-4692", + "CVE-2016-4743", + "CVE-2016-7578", + "CVE-2016-7583", + "CVE-2016-7586", + "CVE-2016-7587", + "CVE-2016-7589", + "CVE-2016-7592", + "CVE-2016-7598", + "CVE-2016-7599", + "CVE-2016-7610", + "CVE-2016-7611", + "CVE-2016-7614", + "CVE-2016-7632", + "CVE-2016-7635", + "CVE-2016-7639", + "CVE-2016-7640", + "CVE-2016-7641", + "CVE-2016-7642", + "CVE-2016-7645", + "CVE-2016-7646", + "CVE-2016-7648", + "CVE-2016-7649", + "CVE-2016-7652", + "CVE-2016-7654", + "CVE-2016-7656", + "CVE-2017-2383", + }, continuestoUpdate: true, }, } @@ -219,12 +257,16 @@ func TestTranslateCPEToCVE(t *testing.T) { // Given that new vulnerabilities can be found on these // packages/products, we check that at least the // known ones are found. - for _, cve := range tt.cves { + for _, cve := range tt.includedCVEs { require.Contains(t, cvesFound, cve, tt.cpe) } } else { // Check for exact match of CVEs found. - require.ElementsMatch(t, cvesFound, tt.cves, tt.cpe) + require.ElementsMatch(t, cvesFound, tt.includedCVEs, tt.cpe) + } + + for _, cve := range tt.excludedCVEs { + require.NotContains(t, cvesFound, cve, tt.cpe) } require.True(t, ds.DeleteOutOfDateVulnerabilitiesFuncInvoked) diff --git a/server/vulnerabilities/oval/parsed/dpkg_infotest.go b/server/vulnerabilities/oval/parsed/dpkg_infotest.go index 059322c8b1..ce1ff7eb96 100644 --- a/server/vulnerabilities/oval/parsed/dpkg_infotest.go +++ b/server/vulnerabilities/oval/parsed/dpkg_infotest.go @@ -15,7 +15,7 @@ type DpkgInfoTest struct { StateMatch StateMatchType } -// Eval evaluates the given dpkg info test againts a host's installed packages. +// Eval evaluates the given dpkg info test against a host's installed packages. // If test evaluates to true, returns all Software involved with the test match, otherwise will // return nil. func (t *DpkgInfoTest) Eval(packages []fleet.Software) ([]fleet.Software, error) { diff --git a/server/vulnerabilities/oval/parsed/object_state_string.go b/server/vulnerabilities/oval/parsed/object_state_string.go index d10c5d0598..848f85e4b6 100644 --- a/server/vulnerabilities/oval/parsed/object_state_string.go +++ b/server/vulnerabilities/oval/parsed/object_state_string.go @@ -18,7 +18,7 @@ func (sta ObjectStateString) unpack() (OperationType, string) { return NewOperationType(parts[0]), parts[1] } -// Eval evaluates the provided value againts the encoded value in sta according to the encoded +// Eval evaluates the provided value against the encoded value in sta according to the encoded // operation. func (sta ObjectStateString) Eval(other string) (bool, error) { op, val := sta.unpack() diff --git a/server/vulnerabilities/oval/parsed/result.go b/server/vulnerabilities/oval/parsed/result.go index 234a88e79d..aa408b3b66 100644 --- a/server/vulnerabilities/oval/parsed/result.go +++ b/server/vulnerabilities/oval/parsed/result.go @@ -3,7 +3,7 @@ package oval_parsed import "github.com/fleetdm/fleet/v4/server/fleet" type Result interface { - // Eval evaluates the current OVAL definition againts an OS version and a list of installed software, returns all software + // Eval evaluates the current OVAL definition against an OS version and a list of installed software, returns all software // vulnerabilities found. Eval(fleet.OSVersion, []fleet.Software) ([]fleet.SoftwareVulnerability, error) } diff --git a/server/vulnerabilities/oval/parsed/rpm_infotest.go b/server/vulnerabilities/oval/parsed/rpm_infotest.go index a228278e8a..75797435f2 100644 --- a/server/vulnerabilities/oval/parsed/rpm_infotest.go +++ b/server/vulnerabilities/oval/parsed/rpm_infotest.go @@ -12,7 +12,7 @@ type RpmInfoTest struct { StateMatch StateMatchType } -// Eval evaluates the given test againts a host's installed packages. +// Eval evaluates the given test against a host's installed packages. // If test evaluates to true, returns all Software involved with the test match, otherwise will // return nil. func (t *RpmInfoTest) Eval(packages []fleet.Software) ([]fleet.Software, error) {