Skip to content

fix: allow optional json. prefix in rule 932171 ARGS_NAMES pattern#4703

Merged
fzipi merged 2 commits into
coreruleset:mainfrom
fzipi:fix/932171-json-args-names-prefix
Jul 13, 2026
Merged

fix: allow optional json. prefix in rule 932171 ARGS_NAMES pattern#4703
fzipi merged 2 commits into
coreruleset:mainfrom
fzipi:fix/932171-json-args-names-prefix

Conversation

@fzipi

@fzipi fzipi commented Jul 11, 2026

Copy link
Copy Markdown
Member

what

Rule 932171 (Shellshock, CVE-2014-6271) targets ARGS_NAMES|ARGS|FILES_NAMES with the start-anchored pattern ^\(\s*\)\s+\{. The regex now allows an optional json. prefix: ^(?:json\.)?\(\s*\)\s+\{.

why

libModSecurity3/Coraza prefix ARGS_NAMES with json. for parameters extracted from a JSON request body, so ARGS_NAMES for a JSON key like () { :; } becomes json.() { :; }. The ^ anchor in 932171 never matched that prefixed name, meaning the rule missed Shellshock payloads submitted as JSON body keys on those engines. Sibling RCE rules in the same file (932250, 932260, 932236, etc.) already handle this exact case via an optional (?:json\.)? prefix; 932171 was the one rule in the family that hadn't been updated. crs-linter flags this class of issue.

refs

  • consistent with existing fixes for 934170, 943110, 943120, 942220, 942550, 932250, etc.

ai disclosure

  • tools used: Claude (Sonnet 5)
  • assisted with: identifying that the crs-linter warning on rule 932171 was a genuine gap (not a false positive) by cross-referencing sibling RCE rules that already handle the json. ARGS_NAMES prefix; authoring regex-assembly/932171.ra; regenerating the rule regex via crs-toolchain; writing the new go-ftw regression test case; drafting this PR description
  • review performed: ran crs-linter against the full rule set and confirmed the 932171 finding is gone with zero new errors; ran the full REQUEST-932-APPLICATION-ATTACK-RCE go-ftw suite (935 tests) against both modsec2-apache and modsec3-nginx, confirming all pass; manually reverted the fix and re-ran the new test against modsec3-nginx to confirm it fails without the change (proving the bug is real and the test catches it), then restored the fix and re-confirmed all tests pass

libModSecurity3/Coraza prefix ARGS_NAMES with "json." for JSON body
parameters, so the start-anchored Shellshock pattern in 932171 never
matched JSON body key names on those engines. Sibling RCE rules in the
same file (932250, 932260, etc.) already account for this via an
optional (?:json\.)? prefix.
@fzipi
fzipi requested a review from EsadCetiner July 11, 2026 21:09
@github-actions

Copy link
Copy Markdown
Contributor

📊 Quantitative test results for language: eng, year: 2023, size: 10K, paranoia level: 1:
🚀 Quantitative testing did not detect new false positives

EsadCetiner
EsadCetiner previously approved these changes Jul 11, 2026
Comment thread tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932171.yaml Outdated
@fzipi
fzipi requested a review from a team July 13, 2026 02:35
@fzipi
fzipi added this pull request to the merge queue Jul 13, 2026
Merged via the queue into coreruleset:main with commit 0015dbe Jul 13, 2026
8 checks passed
@fzipi
fzipi deleted the fix/932171-json-args-names-prefix branch July 13, 2026 02:36
fzipi added a commit that referenced this pull request Jul 20, 2026
…4728)

Partial cherry-pick of #4703. That PR's actual json.-prefix fix for
ARGS_NAMES is already present on this branch: backporting PR #4672
earlier took the LTS side wholesale for this file and silently
dropped the fix for hand-written inline rules (932171, 942361, see
d9eda15), which was then independently restored on this branch as
part of preparing v4.25.1 - before #4703 re-fixed the same gap on
main (where it had regressed separately via an unrelated fork-merge
conflict resolution mistake).

So the only real delta here is formalizing 932171 into a proper
regex-assembly file (previously hand-written inline) and applying
the project's standard [\s\x0b] whitespace-class normalization -
a consistency/maintainability change, not a functional fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants