Merge pull request #323 from SigmaHQ/321-filters-sigma-filter-selecti…

…on-groups-dont-seem-to-perform-exact-matching

[Sigma Filters] Fixes a bug in condition matching logic

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

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_"'
+    ]