[high] Parenthesise AND groups under OR (Splunk search evaluates OR before AND)

[elhoim]

BLUF

• The Splunk search command evaluates OR before AND, so an AND group placed under an OR without parentheses changes the query's meaning. Since fix: OR/AND precedence inversion widens emitted queries in SPL and SPL2 backends #77, 1 of selection_* with multi-field selections is emitted as A B OR C D, which Splunk reads as A AND (B OR C) AND D.
• 491 of 3140 SigmaHQ rules have an AND group directly under an OR. On main every one of them is emitted ungrouped. Example: azure_mfa_interrupted now misses both of its intended detections.
• Fix: always parenthesise an AND nested in an OR. fix: OR/AND precedence inversion widens emitted queries in SPL and SPL2 backends #77's OR-under-AND grouping stays. The output is then correct under either precedence reading, so this does not simply revert fix: OR/AND precedence inversion widens emitted queries in SPL and SPL2 backends #77.
• Also: leading terms are moved in front of the rex/eval pipeline only when the search expression has no top-level OR. This stops (f=1 and x|re) or h=3 from turning into f=1 AND (x OR h=3). It affected 3 SigmaHQ rules; after the fix, none.

Priority: high

Details

Splunk's documented evaluation order

From the Splunk Search Reference, search command, Boolean expressions:

The order in which Boolean expressions are evaluated with the search command is:

1. Expressions within parentheses
2. NOT clauses
3. OR clauses
4. AND clauses

This evaluation order is different than the order used with the eval and where commands, which evaluate AND before OR clauses.

Relation to #73 / #77

Thanks to @cristianchiriac for #73/#77. They were right that an OR nested inside an AND must be parenthesised, and that part stays as it is. #77 did this by changing the precedence tuple to (NOT, AND, OR). That tuple also tells pySigma that an AND nested inside an OR needs no parentheses, and under the documented search order it does. So #77 fixed one direction and broke the other: test_splunk_or_and_expression was changed from (fieldA="valueA1" fieldB="valueB1") OR (…) to fieldA="valueA1" fieldB="valueB1" OR ….

This PR keeps #77's tuple and overrides compare_precedence so that an AND under an OR is always grouped. Every mixed AND/OR nesting is now parenthesised, which is correct whichever order Splunk actually uses. Queries with a single operator (such as a b c or a OR b OR c), and NOT, are unchanged.

I also considered parenthesize = True. It is equally correct, but it wraps every NOT and changes 42 existing test expectations. The targeted override adds parentheses only where the two readings disagree.

The extended correlation condition (temporal with and/or) is also rendered into a | search, so CorrelationConditionAND under CorrelationConditionOR is grouped as well. SPL2 (spl2.py) emits explicit AND/OR tokens and is not touched.

Example: SigmaHQ azure_mfa_interrupted.yml

selection_50074:
    ResultType: 50074
    ResultDescription|contains: 'Strong Auth required'
selection_500121:
    ResultType: 500121
    ResultDescription|contains: 'Authentication failed during strong authentication request'
condition: 1 of selection_*

Before (main):

ResultType=50074 ResultDescription="*Strong Auth required*" OR ResultType=500121 ResultDescription="*Authentication failed during strong authentication request*"

Under the documented order this is ResultType=50074 AND (ResultDescription="*Strong Auth required*" OR ResultType=500121) AND ResultDescription="*Authentication failed during strong authentication request*". A 50074 event now also needs the 500121 description text, and a 500121 event can never match. Neither intended detection fires.

After:

(ResultType=50074 ResultDescription="*Strong Auth required*") OR (ResultType=500121 ResultDescription="*Authentication failed during strong authentication request*")

Corpus impact

I converted the SigmaHQ master rules (3140 rules, no pipeline) with the real backend:

• 491 rules contain an AND group directly under an OR. On main all of them are emitted without grouping.
• 3 rules had a term hoisted out of a disjunction (proc_creation_lnx_file_and_directory_discovery and two macOS rules). After the fix there are 0. The conversion error count is unchanged.

Hoisting in finalize_query_default

The previous guard only checked whether the remaining query starts with OR. For (f=1 and x|re) or h=3 the output on main is:

f="1"
| rex field=x ...
| eval xCondition=...
| search xCondition="true" OR h="3"

Here f=1 became a conjunct of the whole disjunction, and that is wrong under either precedence reading. The new _has_top_level_or helper scans the search expression for an OR outside quotes and parentheses, and skips hoisting when it finds one. Hoisting of index=/source= conditions on top-level conjunctions is unchanged; test_splunk_regex_query_explicit_or_with_add_condition still passes.

Changed test expectations

• test_splunk_or_and_expression: back to its pre-fix: OR/AND precedence inversion widens emitted queries in SPL and SPL2 backends #77 grouped form.
• test_splunk_regex_query_explicit_or_with_add_condition: (EventID=4688 CommandLineCondition="true" OR ImageCondition="true") becomes ((EventID=4688 CommandLineCondition="true") OR ImageCondition="true"). Under the documented order, the old form required EventID=4688 for the Image branch too.

Testing

• New tests:
  • AND nested in OR, using the azure_mfa_interrupted shape
  • mixed nesting (a and (b or c)) or d
  • no hoisting out of a grouped disjunction
  • no hoisting out of an ungrouped disjunction (the hoisting guard on its own)
  • _has_top_level_or cases, including quoted OR and escaped quotes
  • an extended temporal correlation condition (r1 and r2) or r3
• Without the fix, 14 tests fail. With it, the full suite passes: pytest 151 passed, with Python 3.12 and pySigma 1.2.0 from poetry.lock, run the same way as CI (poetry install + pytest).
• black==24.1.1 (pre-commit pin) reports nothing on the changed code.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟢 Approval recommended

The semantic fixes are narrowly scoped and backed by comprehensive new/updated tests, with only a minor test robustness improvement suggested.

Review effort: Lite
Findings: 1

Open (1)

• Brittle test depends on exact whitespace formatting · New

What changed in this PR

This PR fixes incorrect boolean grouping in the classic Splunk SPL backend where Splunk search evaluates OR before implicit AND, which can change rule semantics when an AND-group is emitted directly under an OR. It also tightens the “hoisting” optimization so leading terms aren’t moved ahead of deferred rex/eval pipelines when the remaining search expression contains a top-level disjunction.

Changes:

• Override precedence comparison to always parenthesize AND when nested under OR (including in extended correlation conditions rendered via | search).
• Add _has_top_level_or() and use it to prevent hoisting terms out of disjunction branches in finalize_query_default.
• Update and add regression tests covering AND/OR nesting, hoisting behavior, and _has_top_level_or parsing cases.

File	Description
sigma/​backends/​splunk/​splunk.py	Adds targeted grouping override and a top-level OR detector to keep emitted SPL semantically correct under Splunk search precedence and to prevent unsafe hoisting across pipelines.
tests/​test_backend_splunk.py	Updates expectations and adds tests for AND-in-OR grouping, mixed nesting, hoisting guard behavior, and _has_top_level_or.
tests/​test_backend_splunk_correlations.py	Adds a regression test ensuring extended temporal correlation conditions also group AND nested under OR in the trailing `

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.