fix: 4RI-250413 by EsadCetiner · Pull Request #4672 · coreruleset/coreruleset

EsadCetiner · 2026-06-20T09:41:50Z

Proposed changes

Fixes 4RI-250413

PR Checklist

I have read the CONTRIBUTING doc
I have added positive tests proving my fix/feature works as intended.
I have added negative tests that prove my fix/feature considers common cases that might end in false positives
In case you changed a regular expression, you are not adding a ReDOS for pcre. You can check this using regexploit
My test use the comment field to write the expected behavior
I have added documentation for the rule or change (when appropriate)

Further comments

AI Disclosure

AI was not used.

For the reviewer

Positive and negative tests were added
Tests cover the intended fix/feature properly
No usage of dangerous constructs like ctl:requestBodyAccess=Off were used in the rule
In case a regular expression was changed, there is no ReDOS
Documentation is clear for the rule/change
If a contribution shows signs of unreviewed AI generation (e.g., plausible-but-broken regex, generic boilerplate comments, hallucinated SecLang directives), reviewers should ask about AI usage regardless of what was checked.

github-actions · 2026-06-20T09:43:59Z

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

HackingRepo · 2026-06-20T09:53:53Z

👎 1 test(s) failed to run: ["943110-42"], fix it

EsadCetiner · 2026-06-20T09:56:22Z

@HackingRepo I'm aware, please let me work on the tests.

fzipi · 2026-06-22T18:08:59Z

There are two places where the prefix is redundant and one rule from the same issue that was missed.

Problem 1: Redundant fix in `regex-assembly/941170.ra`

The five javascript: and data: patterns in 941170 use (?:\W|^) as their anchor, not a bare ^. The change looks like:

-  (?:\W|^)javascript:.+[=\x5c\(\[\.<]
+  (?:\W|^(?:json\.)?)javascript:.+[=\x5c\(\[\.<]

This is redundant. The json. prefix always ends with a dot (.), which is a non-word character and therefore already matched by the \W branch. Given input json.javascript:alert, the . before javascript: satisfies \W, so the original pattern already fires. The same applies to all five variants in the file (javascript: with special chars, javascript: with name keyword, and the three data: forms).

The added (?:json\.)? inside the ^ alternative is dead weight — it only activates when the input starts with json. at position 0 AND would not have been caught by \W, which cannot happen because json. always supplies a \W character before the attack token.

Request: revert all five changes in 941170.ra...and maybe add a comment that json. would already be catched?

Problem 2: Redundant fix in `regex-assembly/921200.ra`

The LDAP rule uses a ##!^ prefix:

-##!^ ^[^:\(\)&\|\!\x3c\x3e~]*\)\s*
+##!^ ^(?:json\.)?[^:\(\)&\|\!\x3c\x3e~]*\)\s*

The character class [^:\(\)&\|\!\x3c\x3e~] excludes :, (, ), &, |, !, <, >, ~. It does not exclude j, s, o, n, or .. So a value like json.something) is already matched by ^[^...]*\) — the json. substring passes the class, and ) closes the match.

Adding ^(?:json\.)? before a pattern whose character class already admits every character in json. has no effect.

Request: revert the change in 921200.ra, same maybe adding a comment?

Problem 3: Missing fix for rule 942522

Rule 942522 (PL2, SQLi) has a ^-anchored pattern:

^.*?\x5c['\"`](?:.*?['\"`])?\s*(?:and|or)\b

This is the same class of problem as the other fixed rules: on libModSec3, a JSON key that would trigger this rule is exposed as json.<key> in ARGS_NAMES, and the bare ^ causes the match to fail. This rule was not updated in the PR.

Request: add (?:json\.)? after ^ in the 942522 pattern (and regenerate the conf file).

Minor point

Negative tests for the fix are missing (noted in the PR checklist). A test showing that a benign JSON key like json.username does not trigger session fixation rules (943110/943120) would confirm the fix does not widen detection unintentionally.

EsadCetiner · 2026-06-23T10:30:49Z

@fzipi Applied your suggestions, however point 3 is wrong because there's a .* after the anchor which effectively will match anything up to \x5c. That regex is probably wrong for including .*, but that's out of scope for this PR.

Also, we should be adding disclaimers to all AI generated messages.

for more information, see https://pre-commit.ci

Signed-off-by: Felipe Zipitria <[email protected]>

fzipi · 2026-06-26T08:34:28Z

I went ahead and rewrote the regex-assembly/942360.ra file as full regex-assembly, keeping the output equivalent.

Rule 942362 uses a ^[\W\d]+ anchor that silently fails on libModSecurity3 because JSON payloads prepend json. (starting with j, a letter) to ARGS_NAMES. Apply the same fix used in 942360/942361: ^(?:json\.)?[\W\d]+.

fzipi · 2026-06-26T09:11:45Z

while reviewing the coverage, i found that rule 942362 was missing the same fix applied to its sibling rules.

its .ra file defined {{nonword_number}} as ^[\W\d]+\s*? — the ^[^A-Z_a-z]+ anchor silently fails on libmodsecurity3 because json. starts with j (a letter), so any json parameter name carrying a sqli payload like "; select" would bypass detection at pl2.

i applied the same fix used in 942360 and 942361: ^(?:json\.)?[\W\d]+\s*?, regenerated the rule with crs-toolchain, and added a test case (942362 test 33) covering the json args_names path. all tests pass.

pushed to the branch.

this comment was drafted with assistance from claude sonnet 4.6

fzipi

LGTM. Got all the rules from the failing check in https://github.com/coreruleset/crs-linter/actions/runs/28227921479/job/83624292843?pr=155, proving the check works 😉

Co-authored-by: Max Leske <[email protected]>

…verride Cherry-picking PR #4672 (4RI-250413) took the LTS side wholesale for rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf and rules/REQUEST-942-APPLICATION-ATTACK-SQLI.conf, then only regenerated the rule IDs backed by a regex-assembly (.ra) file. Two hand-written inline regexes with no .ra file (932171, 942361) were silently dropped in that process and never got the `(?:json\.)?` ARGS_NAMES prefix fix, so 942361-9 failed to detect the SQLi payload on nginx/libmodsecurity3. Also updates the nginx-overrides.yaml entry for 920100 test 4: the libmodsecurity v3.0.16 image bump also picked up nginx 1.30.3 (up from 1.28.2), which now rejects CONNECT with 405 instead of 400.

* fix: false positive with parameter name `.history` (#4614) (cherry picked from commit 0885958) * fix(942200): reduce false positives on payloads with comments (#4608) * fix(942200): reduce false positives on payloads with comments * add known fp test case * improve test comments * fix: wrong test * add known fp test case * fix typo * fix typo (cherry picked from commit 2c64b25) * fix(unix): exclude `pg` command from pl-1 (#4613) (cherry picked from commit 82195a4) * fix(920100): drop HTTP/0.9 GET support from request line validation (#4621) * fix(920100): drop HTTP/0.9 GET support from request line validation remove the GET-without-protocol alternative from the regex assembly. HTTP/0.9 is obsolete per RFC 9110 and the dedicated alternative was the only one that did not require a protocol suffix in the request line. GET requests with proper HTTP/1.x or HTTP/2 protocol versions remain covered by the generic method alternative. * Apply suggestions from code review Co-authored-by: Max Leske <[email protected]> --------- Co-authored-by: Max Leske <[email protected]> (cherry picked from commit f8402a9) * fix(920240, 920400): don't rely on content-type header (#4639) (cherry picked from commit 2f1afb7) * fix: remove exponential backtracking in 933160/933161 comment suffix (#4666) * fix: remove exponential backtracking in 933160/933161 comment suffix The suffix that skips whitespace and PHP comments between a function name and the opening parenthesis used an ambiguous alternation where the whitespace branch and the comment bodies both matched the same characters under a `*` quantifier. On a backtracking engine a payload that never reaches `(` backtracks exponentially. De-ambiguate the alternation while keeping RE2 compatibility (no atomic groups): - block comments use the unrolled form instead of `/\*.*\*/` - line comments (`#`, `//`) must terminate at a line break, because a line comment before `(` is only reachable across a newline; otherwise the `(` is inside the comment and there is no function call Add positive tests for the comment/whitespace evasions (block, line with newline, multi-line block, vertical tab, mixed) and negatives to 933161. * docs: explain unrolled comment form in 933160/933161 .ra * Update regex-assembly/933161.ra Co-authored-by: Max Leske <[email protected]> --------- Co-authored-by: Max Leske <[email protected]> (cherry picked from commit c1df902) * fix(941140): remove exponential backtracking in CSS url(javascript) detection (#4670) * fix(941140): bound CSS declaration value to remove exponential backtracking Move the inline pattern to regex-assembly/941140.ra and replace the free '.+' in the declaration list with '[^;]+', removing the ambiguity that caused exponential backtracking on backtracking engines (PCRE2/RE2 were unaffected). Detection is unchanged (differential-tested). * docs(941140): clarify whitespace handling and regression-test intent Address review feedback on #4670: - note that t:removeWhitespace strips whitespace before the regex runs, so the character classes intentionally do not account for it - reword test_id 13 desc: it is a no-match canary for the unbounded-value shape (PCRE2 tames both old and new patterns), not a latency assertion (cherry picked from commit fe59387) * fix(933180): remove exponential backtracking in variable-function noise suffix (#4669) * fix(933180): remove exponential backtracking in variable-function noise suffix * fix(933180): require non-empty array/brace bodies in noise suffix Address review on #4669: use + instead of * for the array-access and curly-brace noise branches so empty [] / {} are not matched. This restores the non-empty semantics of the original .+ patterns; the * form had accidentally broadened them. Differential vs the original rule shows zero true-positive loss; REQUEST-933 416/416 pass. * test(933180): verify empty array/brace bodies are not matched (cherry picked from commit 3828246) * feat(crs-setup): add default actions for phases 3-5 (#4675) (cherry picked from commit 09c1a11) * fix: 4RI-250413 (#4672) * fix: 4RI-250413 * fix: tests * chore(formatting): auto fixes from pre-commit hooks for more information, see https://pre-commit.ci * fix: typo * fix: tests * apply code review suggestions * apply code review suggestions * chore(formatting): auto fixes from pre-commit hooks for more information, see https://pre-commit.ci * chore(942360): update regex with better format Signed-off-by: Felipe Zipitria <[email protected]> * fix(942362): add json. prefix support for JSON ARGS_NAMES Rule 942362 uses a ^[\W\d]+ anchor that silently fails on libModSecurity3 because JSON payloads prepend json. (starting with j, a letter) to ARGS_NAMES. Apply the same fix used in 942360/942361: ^(?:json\.)?[\W\d]+. * Apply suggestions from code review Co-authored-by: Max Leske <[email protected]> * apply code review suggestions * Update regex-assembly/942360.ra Co-authored-by: Max Leske <[email protected]> --------- Signed-off-by: Felipe Zipitria <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Felipe Zipitria <[email protected]> Co-authored-by: Max Leske <[email protected]> (cherry picked from commit 6e9ff3c) * chore: release v4.25.1 * fix: correct indentation and action order in commented 900510 example * docs: note libmodsecurity v3.0.16 requirement in CHANGES * chore(tests): bump modsecurity-crs nginx image to libmodsecurity v3.0.16 * fix(901180): split XML attribute inspection gate to fix unused TX variable lint error * fix: renumber duplicated test Signed-off-by: Felipe Zipitria <[email protected]> * fix: restore dropped json. prefix fixes and fix stale nginx CONNECT override Cherry-picking PR #4672 (4RI-250413) took the LTS side wholesale for rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf and rules/REQUEST-942-APPLICATION-ATTACK-SQLI.conf, then only regenerated the rule IDs backed by a regex-assembly (.ra) file. Two hand-written inline regexes with no .ra file (932171, 942361) were silently dropped in that process and never got the `(?:json\.)?` ARGS_NAMES prefix fix, so 942361-9 failed to detect the SQLi payload on nginx/libmodsecurity3. Also updates the nginx-overrides.yaml entry for 920100 test 4: the libmodsecurity v3.0.16 image bump also picked up nginx 1.30.3 (up from 1.28.2), which now rejects CONNECT with 405 instead of 400. * docs: note ModSecurity v2 limitation for XML attribute opt-out in CHANGES --------- Signed-off-by: Felipe Zipitria <[email protected]> Co-authored-by: Esad Cetiner <[email protected]> Co-authored-by: Max Leske <[email protected]> Co-authored-by: Prateek saini <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…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.

EsadCetiner added the release:fix label Jun 20, 2026

EsadCetiner added the backport:lts-4.25.1 PR that must be backported to LTS release 4.25.1 label Jun 20, 2026

EsadCetiner mentioned this pull request Jun 20, 2026

Feature: Require (?:json\.)? prefix when targeting ARGS_NAMES and anchoring to start/full-string coreruleset/crs-linter#154

Open

fzipi force-pushed the 4RI-250413 branch from a9c70d2 to 8fce7ee Compare June 22, 2026 12:54

fzipi reviewed Jun 22, 2026

View reviewed changes

Comment thread regex-assembly/934170.ra Outdated

EsadCetiner and others added 7 commits June 25, 2026 11:56

fix: 4RI-250413

d7695fd

fix: tests

8fa17ae

chore(formatting): auto fixes from pre-commit hooks

c4183b5

for more information, see https://pre-commit.ci

fix: typo

72189b6

fix: tests

4363236

apply code review suggestions

58957eb

apply code review suggestions

9a09174

fzipi force-pushed the 4RI-250413 branch from 3df0006 to 9a09174 Compare June 25, 2026 09:56

EsadCetiner and others added 3 commits June 26, 2026 10:14

Merge branch 'main' into 4RI-250413

91d6fa2

chore(formatting): auto fixes from pre-commit hooks

b36880c

for more information, see https://pre-commit.ci

chore(942360): update regex with better format

85d0e7e

Signed-off-by: Felipe Zipitria <[email protected]>

fzipi mentioned this pull request Jun 26, 2026

feat: add lint rule for ARGS_NAMES start-anchored operators missing json prefix coreruleset/crs-linter#155

Open

3 tasks

fix(942362): add json. prefix support for JSON ARGS_NAMES

49355fb

Rule 942362 uses a ^[\W\d]+ anchor that silently fails on libModSecurity3 because JSON payloads prepend json. (starting with j, a letter) to ARGS_NAMES. Apply the same fix used in 942360/942361: ^(?:json\.)?[\W\d]+.

fzipi previously approved these changes Jun 26, 2026

View reviewed changes

EsadCetiner requested a review from a team June 27, 2026 23:30

theseion requested changes Jun 28, 2026

View reviewed changes

Comment thread regex-assembly/942360.ra Outdated

Comment thread regex-assembly/942360.ra Outdated

Comment thread tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932171.yaml Outdated

Apply suggestions from code review

b57eaf2

Co-authored-by: Max Leske <[email protected]>

EsadCetiner dismissed fzipi’s stale review via b57eaf2 June 28, 2026 07:15

apply code review suggestions

8d05d5c

EsadCetiner requested a review from theseion June 28, 2026 07:18

theseion requested changes Jun 28, 2026

View reviewed changes

Comment thread regex-assembly/942360.ra Outdated

Update regex-assembly/942360.ra

b70929a

Co-authored-by: Max Leske <[email protected]>

EsadCetiner requested a review from theseion June 28, 2026 07:26

theseion approved these changes Jun 29, 2026

View reviewed changes

EsadCetiner added this pull request to the merge queue Jun 29, 2026

Merged via the queue into coreruleset:main with commit 6e9ff3c Jun 29, 2026
9 of 10 checks passed

EsadCetiner deleted the 4RI-250413 branch June 29, 2026 04:07

fzipi mentioned this pull request Jul 1, 2026

chore: release v4.25.1 #4688

Merged

theseion mentioned this pull request Jul 6, 2026

Monthly Chat Agenda July 2026 (2026-07-06) #4701

Open

This was referenced Jul 19, 2026

fix(932171): backport regex-assembly formalization to lts/v4.25.x #4727

Closed

fix(932171): backport regex-assembly formalization to lts/v4.25.x #4728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: 4RI-250413#4672

fix: 4RI-250413#4672
EsadCetiner merged 14 commits into
coreruleset:mainfrom
EsadCetiner:4RI-250413

EsadCetiner commented Jun 20, 2026

Uh oh!

github-actions Bot commented Jun 20, 2026 •

edited

Loading

Uh oh!

HackingRepo commented Jun 20, 2026 •

edited

Loading

Uh oh!

EsadCetiner commented Jun 20, 2026

Uh oh!

Uh oh!

fzipi commented Jun 22, 2026

Uh oh!

EsadCetiner commented Jun 23, 2026

Uh oh!

fzipi commented Jun 26, 2026

Uh oh!

fzipi commented Jun 26, 2026

Uh oh!

fzipi left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

EsadCetiner commented Jun 20, 2026

Proposed changes

PR Checklist

Further comments

AI Disclosure

For the reviewer

Uh oh!

github-actions Bot commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HackingRepo commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EsadCetiner commented Jun 20, 2026

Uh oh!

Uh oh!

fzipi commented Jun 22, 2026

Problem 1: Redundant fix in regex-assembly/941170.ra

Problem 2: Redundant fix in regex-assembly/921200.ra

Problem 3: Missing fix for rule 942522

Minor point

Uh oh!

EsadCetiner commented Jun 23, 2026

Uh oh!

fzipi commented Jun 26, 2026

Uh oh!

fzipi commented Jun 26, 2026

Uh oh!

fzipi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jun 20, 2026 •

edited

Loading

HackingRepo commented Jun 20, 2026 •

edited

Loading

Problem 1: Redundant fix in `regex-assembly/941170.ra`

Problem 2: Redundant fix in `regex-assembly/921200.ra`