Add nil checks for slice pointers; filter invalid overrides from reports (#1031)

* Add nil checks for slice pointers; filter invalid overrides from reports

Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>

* Re-word comment

Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>

---------

Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>

Author: egibs

pkg/report/report.go

@@ -448,7 +448,11 @@ func Generate(ctx context.Context, path string, mrs *yarax.ScanResults, c malcon
 
 		b := buildBehavior(m, matchedStrings, key, ruleURL, risk)
 
-		handleMetadata(m, b, fr, override, mrsMap, &pledges, &caps, &syscalls)
+		// if the rule has an override tag but is not overriding a valid rule,
+		// ignore this match rule so that we don't show errant false positive rules in reports
+		if !parseMetadata(m, b, fr, override, mrsMap, &pledges, &caps, &syscalls) {
+			continue
+		}
 
 		// Fix YARA Forge rules that record their author URL as reference URLs
 		if strings.HasPrefix(b.RuleURL, b.ReferenceURL) {
@@ -476,8 +480,6 @@ func Generate(ctx context.Context, path string, mrs *yarax.ScanResults, c malcon
 		}
 
 		updateBehavior(fr, b, key)
-
-		// TODO: If we match multiple rules within a single namespace, merge matchstrings
 	}
 
 	// Update the behaviors to account for overrides
@@ -589,10 +591,14 @@ func buildBehavior(m *yarax.Rule, matchedStrings []string, key string, ruleURL s
 	}
 }
 
-func handleMetadata(m *yarax.Rule, b *malcontent.Behavior, fr *malcontent.FileReport, override bool, mrsMap map[string]*yarax.Rule, pledges *[]string, caps *[]string, syscalls *[]string) {
+func parseMetadata(m *yarax.Rule, b *malcontent.Behavior, fr *malcontent.FileReport, override bool, mrsMap map[string]*yarax.Rule, pledges *[]string, caps *[]string, syscalls *[]string) bool {
 	k := ""
 	v := ""
 
+	// valid represents whether a rule's metadata contains a legitimate override
+	// or is otherwise valid for the matching rule
+	valid := true
+
 	for _, meta := range m.Metadata() {
 		k = meta.Identifier()
 		v = fmt.Sprintf("%s", meta.Value())
@@ -601,25 +607,6 @@ func handleMetadata(m *yarax.Rule, b *malcontent.Behavior, fr *malcontent.FileRe
 			continue
 		}
 
-		// If we find a match in the map for the metadata key, that's the rule to override
-		// Store this rule (the override) in the fr.Overrides behavior slice
-		// If an override rule is not overriding a valid rule, log an error
-		_, exists := mrsMap[k]
-		switch {
-		case exists && override:
-			var overrideSev int
-			if sev, ok := Levels[v]; ok {
-				overrideSev = sev
-			}
-			b.RiskLevel = RiskLevels[overrideSev]
-			b.RiskScore = overrideSev
-			b.Override = append(b.Override, k)
-			fr.Overrides = append(fr.Overrides, b)
-		case !exists && override:
-			// TODO: return error if override references an unknown rule name
-			continue
-		}
-
 		switch k {
 		case "author":
 			b.RuleAuthor = v
@@ -656,14 +643,47 @@ func handleMetadata(m *yarax.Rule, b *malcontent.Behavior, fr *malcontent.FileRe
 			// YARAforge forgets to encode spaces
 			b.RuleURL = fixURL(v)
 		case "pledge":
-			*pledges = append(*pledges, v)
+			// pledges should not be nil when we get here, but guard against it
+			if pledges != nil {
+				*pledges = append(*pledges, v)
+			}
 		case "syscall":
-			sy := strings.Split(v, ",")
-			*syscalls = append(*syscalls, sy...)
+			// syscalls should not be nil when we get here, but guard against it
+			if syscalls != nil {
+				calls := strings.Split(v, ",")
+				*syscalls = append(*syscalls, calls...)
+			}
 		case "cap":
-			*caps = append(*caps, v)
+			// caps should not be nil when we get here, but guard against it
+			if caps != nil {
+				*caps = append(*caps, v)
+			}
+		case "filetypes":
+			continue
+		// If we find a match in the map for the metadata key after exhausting known keys, that's the rule to override
+		// Store this rule (the override) in the fr.Overrides behavior slice
+		// If an override rule is not overriding a valid rule, set `valid` to false so we can
+		// skip the parent rule match in the report
+		default:
+			_, exists := mrsMap[k]
+			switch {
+			case exists && override:
+				var overrideSev int
+				if sev, ok := Levels[v]; ok {
+					overrideSev = sev
+				}
+				b.RiskLevel = RiskLevels[overrideSev]
+				b.RiskScore = overrideSev
+				b.Override = append(b.Override, k)
+				fr.Overrides = append(fr.Overrides, b)
+			case !exists && override:
+				valid = false
+				continue
+			}
 		}
 	}
+
+	return valid
 }
 
 func updateBehavior(fr *malcontent.FileReport, b *malcontent.Behavior, key string) {

rules/false_positives/lslogins.yara

@@ -1,7 +1,7 @@
 rule lastlogins: override linux {
   meta:
-    description   = "lastlogins"
-    login_records = "low"
+    description    = "lastlogins"
+    current_logins = "low"
 
   strings:
     $lastlogin = "LAST-LOGIN"

rules/false_positives/slirp.yara

@@ -2,7 +2,6 @@ rule snapd: override linux {
   meta:
     description           = "snapd"
     nohup                 = "medium"
-    login_records         = "medium"
     dev_mem               = "medium"
     dev_mmc               = "medium"
     busybox_runner        = "medium"

rules/false_positives/snapd.yara

@@ -1,9 +1,8 @@
 rule ignore_sshd: override {
   meta:
-    description   = "sshd"
-    login_records = "medium"
-    id_rsa        = "low"
-    sshd          = "low"
+    description = "sshd"
+    id_rsa      = "low"
+    sshd        = "low"
 
   strings:
     $auth    = "SSH_USER_AUTH"

rules/false_positives/ssh.yara

@@ -1,12 +1,11 @@
 rule vmtools: override {
   meta:
-    description                      = "vmtools"
-    backdoor                         = "medium"
-    linux_critical_system_paths_high = "medium"
-    proc_net_route_high              = "medium"
-    proc_s_exe                       = "medium"
-    sys_net_recon_exfil              = "medium"
-    proc_s_cmdline                   = "medium"
+    description         = "vmtools"
+    backdoor            = "medium"
+    proc_net_route_high = "medium"
+    proc_s_exe          = "medium"
+    sys_net_recon_exfil = "medium"
+    proc_s_cmdline      = "medium"
 
   strings:
     $vmtools   = "VMTools" fullword

rules/false_positives/vmtools.yara

@@ -1,4 +1,4 @@
-rule kallsyms_lookup: high linux {
+rule kallsyms_lookup: high {
   meta:
     description = "access unexported kernel symbols"
     ref         = "https://lwn.net/Articles/813350/"
@@ -15,9 +15,10 @@ rule kallsyms_lookup: high linux {
     filesize < 1MB and $ref and none of ($not*)
 }
 
-rule kallsyms: medium linux {
+rule kallsyms: medium {
   meta:
     description = "access kernel symbols"
+    filetypes   = "c,elf,so"
 
   strings:
     $kallsyms = "/proc/kallsyms"
@@ -26,11 +27,10 @@ rule kallsyms: medium linux {
     any of them
 }
 
-rule bpftrace: override linux {
+rule bpftrace: medium {
   meta:
     description = "bpftrace"
     filetypes   = "c,elf,so"
-    kallsyms    = "medium"
 
   strings:
     $ref2 = "BPFTRACE" fullword
@@ -39,7 +39,7 @@ rule bpftrace: override linux {
     filesize < 2MB and any of them
 }
 
-rule bpf: override linux {
+rule bpf: override {
   meta:
     description     = "libbpf"
     filetypes       = "c,so,elf"

rules/persist/kernel_module/symbol-lookup.yara

@@ -1,6 +1,7 @@
 rule sudo_editor: medium {
   meta:
     description = "references /etc/sudoers"
+    filetypes   = "elf,macho,so"
 
   strings:
     $etc_sudoers = "/etc/sudoers"
@@ -15,6 +16,7 @@ rule sudo_editor: medium {
 rule small_elf_sudoer: high {
   meta:
     description = "references /etc/sudoers"
+    filetypes   = "elf,macho,so"
 
   condition:
     uint32(0) == 1179403647 and filesize < 10MB and sudo_editor
@@ -23,6 +25,7 @@ rule small_elf_sudoer: high {
 rule sudo_parser: override {
   meta:
     small_elf_sudoer = "medium"
+    filetypes        = "elf,macho,so"
 
   strings:
     $parse = "sudo_parse"

rules/privesc/sudoers.yara

@@ -7,4 +7,3 @@ net/http/post: medium
 net/socket/connect: medium
 net/socket/send: low
 net/url/embedded: low
-persist/kernel_module/symbol_lookup: low

tests/c/clean/falco/ppm_events.c.simple

@@ -1,7 +1,6 @@
 # javascript/clean/powershell.js: medium
 c2/tool_transfer/os: low
 exec/shell/power: medium
-false-positives/opensearch_dashboard: low
 fs/directory/remove: low
 fs/file/copy: medium
 fs/file/delete: medium