Skip to content

Drop the array marker and use json_meta_contains on sid 5004770 - #729

Merged
CGoggins56 merged 1 commit into
quadrantsec:mainfrom
NRGLine4Sec:fix/array-marker-in-json-key
Sep 11, 2026
Merged

Drop the array marker and use json_meta_contains on sid 5004770#729
CGoggins56 merged 1 commit into
quadrantsec:mainfrom
NRGLine4Sec:fix/array-marker-in-json-key

Conversation

@NRGLine4Sec

Copy link
Copy Markdown

The rule reads

json_meta_content:!".properties.riskEventTypes[]",unfamiliarFeatures; json_contains;

and neither of those two spellings does what it looks like.

src/parsers/json.c builds each stored key with snprintf("%s.%s") and compares it with strcmp, so [] is two characters of the key name and not a marker. The rule names a key no Entra document carries. Its condition is negated, and the engine needs the key to be present to satisfy a negation, so the rule cannot alert at all.

json_contains sets the substring flag for json_content. The json_meta_content branch reads json_meta_contains, so the modifier on this line qualifies nothing and the value stored for the key, which is the serialised array, is compared whole.

Both have to change. Dropping [] alone turns a dead rule into one that alerts on the very risk type it excludes, because the stored value ["unfamiliarFeatures"] is not equal to unfamiliarFeatures.

Measured against Sagan built from 3b9b0fa, one document per row, every other condition of the rule satisfied:

properties.riskEventTypes ships [] only patched
["unfamiliarFeatures"] silent alert silent
["unfamiliarFeatures","anonymized..."] silent alert silent
["anonymizedIPAddress"] silent alert alert
absent silent silent silent
a key spelled riskEventTypes[] alert - silent

The last row is the only thing the rule matches today: a document whose key name ends in two brackets.

rev incremented.

The rule reads

  json_meta_content:!".properties.riskEventTypes[]",unfamiliarFeatures; json_contains;

and neither of those two spellings does what it looks like.

src/parsers/json.c builds each stored key with snprintf("%s.%s") and
compares it with strcmp, so [] is two characters of the key name and
not a marker. The rule names a key no Entra document carries. Its
condition is negated, and the engine needs the key to be present to
satisfy a negation, so the rule cannot alert at all.

json_contains sets the substring flag for json_content. The
json_meta_content branch reads json_meta_contains, so the modifier on
this line qualifies nothing and the value stored for the key, which is
the serialised array, is compared whole.

Both have to change. Dropping [] alone turns a dead rule into one that
alerts on the very risk type it excludes, because the stored value
["unfamiliarFeatures"] is not equal to unfamiliarFeatures.

Measured against Sagan built from 3b9b0fa, one document per row, every
other condition of the rule satisfied:

  properties.riskEventTypes              ships   [] only  patched
  ["unfamiliarFeatures"]                 silent  alert    silent
  ["unfamiliarFeatures","anonymized..."] silent  alert    silent
  ["anonymizedIPAddress"]                silent  alert    alert
  absent                                 silent  silent   silent
  a key spelled riskEventTypes[]         alert   -        silent

The last row is the only thing the rule matches today: a document whose
key name ends in two brackets.

rev incremented.
@NRGLine4Sec

Copy link
Copy Markdown
Author

One sentence in the commit message is wrong, and it is the sentence describing the mechanism rather than the fix. Posting the correction here rather than force-pushing, so the review history stays readable.

Correction to my own commit message, before anyone spends time on it.

I wrote that the json_contains on this line "qualifies nothing". That is not what the parser does. src/rules.c line 2231:

if ( !strcmp(rulesplit, "json_contains") )
    {
        ...
        rulestruct[counters->rulecount].json_content_strstr[json_content_count-1] = 1;
    }

It sets the substring flag on the last json_content, whatever keywords sit between the two. It does not look at what immediately precedes it, and it has no effect on json_meta_content either way.

Measured on a reduced rule, one document per row:

Rule .K holds Result
json_content:".K","abc"; json_contains; abc match
json_content:".K","abc"; json_contains; abcdef match
json_content:".K","abc"; json_meta_content:".M",xyz; json_contains; abcdef match
json_content:".K","abc"; json_meta_content:".M",xyz; abcdef no match

So in sid 5004770 that modifier is not idle: it makes json_content:".properties.riskState","atRisk" a substring test.

What this changes about the patch, and what it does not:

  • The measurements in the commit message stand. Every row of that table was run against the engine rather than reasoned about, on the rule as it ships and on the rule as patched. The rule cannot alert today, and it behaves as written
    with the patch.

  • The patch has a side effect I did not mention. Replacing json_contains with json_meta_contains also removes the substring test from riskState.
    On the values Entra puts in that field, none, confirmedSafe, remediated, dismissed, atRisk, confirmedCompromised, a substring test for atRisk and an exact test for atRisk select the same set, so I could not measure a difference. It is still a change, and it should have been in the message.

  • If you would rather keep the substring test on riskState, the line can carry both modifiers and they do not interfere:

    json_meta_content:!".properties.riskEventTypes",unfamiliarFeatures; json_contains; json_meta_contains;
    

    Measured, with riskEventTypes holding a type that is not excluded:

    riskState json_meta_contains only both modifiers
    atRisk alerts alerts
    atRiskSuffix silent alerts

    Tell me which you prefer and I will push it.

Everything else in the description is unaffected: the [] is part of the key name, the rule names a key no document carries, its negated condition needs that key to be present, and removing the brackets alone makes it alert on the risk type it excludes.

@CGoggins56
CGoggins56 merged commit 17a00d5 into quadrantsec:main Sep 11, 2026
1 check passed
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