Skip to content

chore: release v4.25.1#4688

Merged
fzipi merged 18 commits into
lts/v4.25.xfrom
release/v4.25.1
Jul 2, 2026
Merged

chore: release v4.25.1#4688
fzipi merged 18 commits into
lts/v4.25.xfrom
release/v4.25.1

Conversation

@fzipi

@fzipi fzipi commented Jul 1, 2026

Copy link
Copy Markdown
Member

what

backports 10 merged PRs labeled backport:lts-4.25.1 onto the lts/v4.25.x line, plus a version/copyright bump and changelog entry for v4.25.1.

security fixes:

bug fixes:

other changes:

additional fixes not tied to a specific backported PR (found and fixed while preparing this release):

  • fixed indentation and action ordering (pass/t/ctl before ver) in the commented-out 900510 example in crs-setup.conf.example, which was already present on this LTS branch and failing crs-linter
  • split rule 901180 (XML attribute inspection opt-in gate) into a default-setter (901180) and a value-check enforcer (901181), matching the existing crs_skip_response_analysis convention, to fix a crs-linter "unused TX variable" error — the original single-rule form only ever did a counter/existence check (&TX:foo) with no later value read, so the linter's usage tracker never marked it used
  • bumped the modsec3-nginx test image in tests/docker-compose.yml to the current owasp/modsecurity-crs:nginx digest (libmodsecurity v3.0.16), since rule 901181's ctl:ruleRemoveTargetByTag=...;XML://@* action depends on the lexer fix from owasp-modsecurity/ModSecurity#3589; documented this as a breaking change (libmodsecurity v3.0.16+ required) in CHANGES.md
  • restored two (?:json\.)? ARGS_NAMES-prefix fixes (rules 932171 and 942361) from fix: 4RI-250413 #4672 that were silently dropped during that PR's conflict resolution: those two rules have hand-written regexes with no regex-assembly/*.ra source, so they were missed when only the .ra-backed rule IDs were regenerated via crs-toolchain
  • fixed a stale tests/regression/nginx-overrides.yaml entry for rule 920100 test 4: the nginx image bump above also picked up nginx core 1.30.3 (from 1.28.2), which now rejects CONNECT requests with 405 instead of 400
  • fixed a duplicated test_id in tests/regression/tests/REQUEST-932-APPLICATION-ATTACK-RCE/932250.yaml left over from the fix: 4RI-250413 #4672 conflict resolution

why

this lts branch has diverged from main with its own security fixes (XML attribute injection coverage, unix-shell-evasion ReDoS hardening), so most of the backported PRs conflicted on regenerated regexes and appended test IDs. conflicts were resolved by regenerating regexes from the merged .ra sources via crs-toolchain regex update rather than hand-merging the generated patterns, and renumbering test IDs sequentially. the additional fixes above surfaced from running crs-linter and the full go-ftw suite (on both apache and nginx) against the resulting branch rather than trusting the cherry-picks alone.

refs

ai disclosure

  • tools used: Claude (Sonnet 5)
  • assisted with: determining cherry-pick order to minimize conflicts, resolving merge conflicts (test ID renumbering, regenerating regexes via crs-toolchain from merged .ra sources while preserving existing XML://@* targets), running crs-toolchain chore update-copyright, drafting the CHANGES.md entry, diagnosing and fixing the crs-linter and nginx regression failures described above, drafting this PR description
  • review performed: verified crs-linter reports the same error count as the unmodified lts/v4.25.x baseline (no new issues introduced); ran the full go-ftw regression suite for every touched rule family (920, 921, 930, 932, 933, 934, 941, 942, 943) on both apache and nginx against freshly restarted containers and confirmed all pass; reconstructed the exact 3-way merge git would have produced for each conflicted .conf file (git merge-file with the true base/ours/theirs) and diffed it line-by-line against the committed result to confirm no other content was silently dropped during conflict resolution; manually verified the nginx CONNECT/405 behavior change against the raw HTTP response before updating the override

EsadCetiner and others added 17 commits July 1, 2026 11:23
* 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)
…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)
…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)
…etection (#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)
…se 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)
* 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)
Signed-off-by: Felipe Zipitria <[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.
@fzipi
fzipi requested a review from a team July 1, 2026 13:50
@fzipi fzipi added 🚀 release A new release is coming release:ignore Ignore for changelog release labels Jul 1, 2026
@fzipi

fzipi commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

@Xhoenix What you found is a polinomial problem. Real ReDoS problems are exponential

@fzipi
fzipi merged commit 3b89d5a into lts/v4.25.x Jul 2, 2026
6 checks passed
@fzipi
fzipi deleted the release/v4.25.1 branch July 2, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:ignore Ignore for changelog release 🚀 release A new release is coming

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants