diff --git a/sigma/filters.py b/sigma/filters.py index 8e383fd..f3faee2 100644 --- a/sigma/filters.py +++ b/sigma/filters.py @@ -194,8 +194,8 @@ def apply_on_rule( # Replace each instance of the original condition name with the new condition name to avoid conflicts filter_condition = re.sub( - rf"[^ ]*{original_cond_name}[^ ]*", - cond_name, + rf"(\s|\(|^){original_cond_name}(\s|$|\))", + r"\1" + cond_name + r"\2", filter_condition, ) rule.detection.detections[cond_name] = condition diff --git a/tests/test_filters.py b/tests/test_filters.py index 793c324..03c531c 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -14,6 +14,7 @@ SigmaFilterConditionError, SigmaFilterError, SigmaFilterRuleReferenceError, + SigmaConditionError, ) from sigma.filters import SigmaFilter, SigmaGlobalFilter from sigma.processing.conditions import LogsourceCondition @@ -292,15 +293,78 @@ def test_sigma_filter_with_multiple_conditions_raises_error(sigma_filter): SigmaFilter.from_dict(sf_copy) -# def test_logsource_subset(sigma_filter, test_backend, rule_collection): -# # Remove logsource.category -# new_filter = sigma_filter.to_dict() -# new_filter["logsource"].pop("category") -# sigma_filter = SigmaFilter.from_dict(new_filter) -# rule_collection.rules += [sigma_filter] -# -# rule_collection.resolve_rule_references() -# -# assert test_backend.convert(rule_collection) == [ -# '(EventID=4625 or EventID2=4624) and not User startswith "adm_"' -# ] +def test_regression_github_issue_321(rule_collection, test_backend, sigma_filter): + sigma_filter.filter = SigmaGlobalFilter.from_dict( + { + "rules": [ + "6f3e2987-db24-4c78-a860-b4f4095a7095", + ], + "filter": {"User|startswith": "adm_"}, + "condition": "not filter_with_suffix", + } + ) + + rule_collection.rules += [sigma_filter] + + with pytest.raises(SigmaConditionError): + test_backend.convert(rule_collection) + + +@pytest.mark.parametrize( + "filter_condition", + [ + "not filter", + "not (filter)", + "not ( filter)", + "not (filter )", + "not ( filter )", + "not ( filter )", + "not ((filter))", + "not (((filter)))", + ], +) +def test_regression_github_issue_321_brackets( + rule_collection, test_backend, sigma_filter, filter_condition +): + sigma_filter.filter = SigmaGlobalFilter.from_dict( + { + "rules": [ + "6f3e2987-db24-4c78-a860-b4f4095a7095", + ], + "filter": {"User|startswith": "adm_"}, + "condition": filter_condition, + } + ) + + rule_collection.rules += [sigma_filter] + + assert test_backend.convert(rule_collection) == [ + '(EventID=4625 or EventID2=4624) and not User startswith "adm_"' + ] + + +@pytest.mark.skip("Decision on whether filters should support selection confusion is pending") +def test_regression_github_issue_321_selection_confusion( + rule_collection, test_backend, sigma_filter +): + """ + This test targets a weird quirk of how we do Filtering, where the filter can just use a + selection condition as a filter condition. It's probably not desired behaviour, as you'd + rarely want to filter on a selection condition, and it implies that every rule referenced + also has to have a selection condition. + """ + sigma_filter.filter = SigmaGlobalFilter.from_dict( + { + "rules": [ + "6f3e2987-db24-4c78-a860-b4f4095a7095", + ], + "filter": {"User|startswith": "adm_"}, + "condition": "not selection", + } + ) + + rule_collection.rules += [sigma_filter] + + assert test_backend.convert(rule_collection) == [ + '(EventID=4625 or EventID2=4624) and not User startswith "adm_"' + ]