diff --git a/condition_parser.go b/condition_parser.go index 1bec0ae..01c567d 100644 --- a/condition_parser.go +++ b/condition_parser.go @@ -2,6 +2,7 @@ package sigma import ( "fmt" + "sort" "strings" ) @@ -301,24 +302,27 @@ func evaluateQuantifier(q condNodeQuantifier, items map[string]*detectionItem, n return result } -// matchDetectionItems returns detection item names matching a pattern. +// matchDetectionItems returns detection item names matching a pattern, sorted +// for determinism. Without the sort, map iteration order made quantifier +// evaluation ("N of selection_*", "all of them") non-deterministic, which +// could reorder — and in turn drop — extracted conditions across runs. func matchDetectionItems(pattern string, items map[string]*detectionItem) []string { + var names []string if pattern == "them" || pattern == "*" { // Match all detection items - names := make([]string, 0, len(items)) + names = make([]string, 0, len(items)) for name := range items { names = append(names, name) } - return names - } - - // Glob matching with * wildcard - var names []string - for name := range items { - if globMatch(pattern, name) { - names = append(names, name) + } else { + // Glob matching with * wildcard + for name := range items { + if globMatch(pattern, name) { + names = append(names, name) + } } } + sort.Strings(names) return names } diff --git a/determinism_test.go b/determinism_test.go new file mode 100644 index 0000000..c350fbe --- /dev/null +++ b/determinism_test.go @@ -0,0 +1,68 @@ +package sigma + +import "testing" + +// TestDeterministicExtraction verifies condition extraction is deterministic +// and does not drop same-field comparison conditions. Previously, map +// iteration order in quantifier matching plus an over-broad OR-merge folded +// `field > a` and `field > b` (from different selections) together +// non-deterministically, dropping one bound across runs. +func TestDeterministicExtraction(t *testing.T) { + rule := ` +title: Same field, different comparison values across selections +logsource: + category: process_creation + product: windows +detection: + selection_a: + DestinationPort|gt: 20067 + Image|endswith: '\a.exe' + selection_b: + DestinationPort|gt: 14362 + Image|endswith: '\b.exe' + condition: 1 of selection_* +` + // Both DestinationPort bounds must always be present, on every run. + for i := 0; i < 50; i++ { + res := ExtractConditions(rule) + vals := map[string]bool{} + for _, c := range res.Conditions { + if c.Field == "DestinationPort" { + vals[c.Value] = true + } + } + if !vals["20067"] || !vals["14362"] { + t.Fatalf("run %d: expected both DestinationPort bounds, got %v", i, vals) + } + } +} + +// TestComparisonsNotMergedToAlternatives verifies ordering comparisons on the +// same field across an OR are kept separate (folding them would drop a bound). +func TestComparisonsNotMergedToAlternatives(t *testing.T) { + rule := ` +title: t +logsource: + category: process_creation + product: windows +detection: + sel1: + Port|gt: 100 + sel2: + Port|gt: 200 + condition: sel1 or sel2 +` + res := ExtractConditions(rule) + var portConds int + for _, c := range res.Conditions { + if c.Field == "Port" { + portConds++ + if len(c.Alternatives) > 0 { + t.Errorf("comparison must not gain alternatives: %+v", c) + } + } + } + if portConds != 2 { + t.Errorf("expected 2 separate Port conditions, got %d", portConds) + } +} diff --git a/postprocess.go b/postprocess.go index 93b0523..2f61219 100644 --- a/postprocess.go +++ b/postprocess.go @@ -43,7 +43,16 @@ func groupORConditions(conditions []Condition) []Condition { return result } +// comparisonOperators are ordering operators for which OR-runs must NOT be +// folded into Alternatives: `f > a OR f > b` is not a membership test over +// {a, b}, so merging would silently drop a bound (and, because the merge is +// position-dependent, make extraction non-deterministic). +var comparisonOperators = map[string]bool{">": true, ">=": true, "<": true, "<=": true} + func sameConditionGroup(a, b Condition) bool { + if comparisonOperators[a.Operator] { + return false + } return strings.EqualFold(a.Field, b.Field) && a.Operator == b.Operator && a.Negated == b.Negated &&