chainguard-dev / malcontent

#supply #chain #attack #detection
Apache License 2.0
445 stars 31 forks source link

Provide override rules that lower the priority of matches #471

Closed tstromberg closed 1 month ago

tstromberg commented 1 month ago

At the moment, malcontent handles false-positives by hiding them. Often times though, we want the result to still show up in analyze, but at a lower priority. For example, we have this rule:

rule bash_history : high {
  meta:
    description = "access .bash_history file"
  strings:
    $ref = ".bash_history" fullword
  condition:
    all of them
}

However, /bin/bash will trigger this, which it's correct to do, but malcontent shouldn't categorize it as high as a known-good program. My naive first thought is adding support for override rules that change the priority of a matching rule:

rule bash_history_override_bash : low {
strings:
  $ref = "BASH_VERSION" fullword
  $ref2 = "BASH_REMATCH" fullword
condition:
  bash_history and filesize < 5MB and all of them  
}

The result would then be a low-priority result that says something like access .bash_history file (bash): combining the original description and the override suffix. Nothing stops us from using this technique to explicitly raise the priority of a match, though we implicitly do that already. This could improve upon the output there.

tstromberg commented 1 month ago

This is related in some respect to #414 - though that is a bit broader.

We could also use this idea to omit false-positive results altogether, by using a tag that would be filtered out by default. For example, something like false-positive or none?

egibs commented 1 month ago

473 contains an attempt to do this.

The rule name has to follow the original_rule_override pattern but there may be a way to also allow variants of overrides (but this will be consistent).

If an override for a rule exists, we'll ignore the original rule and use the override's severity with the new description being the original rule's description appended to the override rule's description.

tstromberg commented 1 month ago

@egibs brought up some good points about handling 3rd party rules to me in a conversation. This also had me thinking that this implementation is inefficient:

Here's a twist on the idea that addresses both:

// unchanged bash_history rule
rule bash_history : high {
  meta:
    description = "access .bash_history file"
    decrease_if = "bash"
    drop_if = ""
  strings:
    $ref = ".bash_history" fullword
  condition:
    all of them
}

rule yes_it_is_really_bash : override {
  meta:
    bash_history = "medium"
    third_party_malware_rule = "false_positive"
  strings:
    $ref = "BASH_VERSION" fullword
    $ref2 = "BASH_REMATCH" fullword
  condition:
    filesize < 5MB and all of them  
}

This shows an example of a single override rule that modifies the priority of two rules: a third-party rule and a local rule. The override does not repeat the logic of the original rules, so we're not doubling up on computation time.

We should generally fix false positives natively in YARA when it's easy to do so, but this mechanism works well for lowering priorities and sharing false-positive logic across YARA files.

My feel is that overrides should live as close to the rule they apply to as possible, but for overrides that apply to multiple files, we could store it in a general place, for example, this one could live in rules/overrides/bash.yara.

Thoughts, @egibs ?

egibs commented 1 month ago

I'm not sure what form this will take in the code but it makes sense. I'll keep thinking this through and see if I can get something workable written out.

egibs commented 1 month ago

@tstromberg -- I overhauled #473 and implemented the gist of what you covered in your examples:

For decrease_if and drop_if, I added support for comma-delimited values. For the former, we can specify the target binary and the desired severity (e.g., decrease_if = "bash,low"). For the latter, we can specify a list of targets (drop_if = "bash,fish,sh").

tstromberg commented 1 month ago

Sorry about that - I forgot I had left the "drop_if/decrease_if" in the original rule before settling on a better way to do this:

// unchanged bash_history rule
rule bash_history : high {
  meta:
    description = "access .bash_history file"
  strings:
    $ref = ".bash_history" fullword
  condition:
    all of them
}

rule yes_it_is_really_bash : override {
  meta:
    bash_history = "medium"
    third_party_malware_rule = "false_positive"
  strings:
    $ref = "BASH_VERSION" fullword
    $ref2 = "BASH_REMATCH" fullword
  condition:
    filesize < 5MB and all of them  
}

With this method, you can leave the original rule unchanged, which is important for third party rules.

egibs commented 1 month ago

Cool, that was the easiest of the three to implement so I'll remove the other two.

Edit: added to a clean PR (#481)