Skip to content

fix: combine matching mapping preconditions - #244

Open
a-bonfim-tech wants to merge 1 commit into
WithSecureOpenSource:masterfrom
a-bonfim-tech:fix/combine-matching-preconditions
Open

a-bonfim-tech wants to merge 1 commit into
WithSecureOpenSource:masterfrom
a-bonfim-tech:fix/combine-matching-preconditions

Conversation

@a-bonfim-tech

Copy link
Copy Markdown

Summary

Fix the remaining multi-match behavior tracked in #107.

When more than one mapping precondition matches the same Sigma rule, Chainsaw currently stores preconditions in a map keyed by rule ID and the later match overwrites the earlier one.

This change preserves every matching precondition for a rule and requires all of them to pass before the Sigma rule is evaluated.

Changes

  • Store matching preconditions as Vec<Expression> per rule ID.
  • Accumulate matching preconditions instead of overwriting earlier ones.
  • Evaluate all matching preconditions with AND semantics.
  • Include fields from every matching precondition during preprocessing.
  • Add regression tests covering:
    • preservation of multiple preconditions;
    • AND behavior for multiple preconditions;
    • unchanged behavior for a single precondition.

Scope

This PR only addresses the multiple-matching-preconditions overwrite behavior discussed in #107. It does not change the mapping schema, Sigma rule parsing, negation syntax, or existing mappings.

Validation

Static diff review and regression tests were added. I was not able to run the Rust test suite locally in the current environment because external dependency resolution via GitHub was unavailable.

@alexkornitzer

Copy link
Copy Markdown
Collaborator

Hi @a-bonfim-tech,

Thanks for raising but #107 is still open because the final piece of work there would be about applying preconditions to a subset of rules. Whereas this PR looks to be about addressing a FIXME I left in while doing other work within that issue. I am not against merging this but I assume we are trying to solve the following case:

extensions:
  preconditions:
    - for:
        logsource.category: process_creation
      filter:
        - Provider: Microsoft-Windows-Sysmon
          int(EventID): 1
    - for:
        logsource.category: process_access
      filter:
        - Provider: Microsoft-Windows-Sysmon
          int(EventID): 2

Are we sure we would want to and these together, in the above IIRC these would never both match as Sigma rules can only have a single category. Assuming we do want to and preconditions, where the for condition could overlap or people had written inefficient ones, the better way to solve that problem would be to directly use the Tau expression so it would be something like this:

let mut preconds = FxHashMap::default();
if let Some(extensions) = &mapping.extensions && let Some(preconditions) = &extensions.preconditions
{
    for (rid, rule) in &rules {
        if let Rule::Sigma(sigma) = rule {
            let mut filters = Vec::with_capacity(preconditions.len());
            for precondition in preconditions {
                if precondition.for_.is_empty() {
                    continue;
                }
                let mut matched = true;
                for (f, v) in &precondition.for_ {
                    match sigma.find(f) {
                        Some(value) => {
                            if value.as_str() != Some(v.as_str()) {
                                matched = false;
                                break;
                            }
                        }
                        None => {
                            matched = false;
                            break;
                        }
                    }
                }
                if matched {
                    filters.push(precondition.filter.clone());
                }
            }
            if !filters.is_empty() {
                preconds.insert(*rid, Expression::BooleanGroup(BoolSym::And, filters));
            }
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants