Skip to content

fix(unix): exclude pg command from pl-1#4613

Merged
fzipi merged 1 commit into
coreruleset:mainfrom
EsadCetiner:fix-remove-pg-command
Apr 18, 2026
Merged

fix(unix): exclude pg command from pl-1#4613
fzipi merged 1 commit into
coreruleset:mainfrom
EsadCetiner:fix-remove-pg-command

Conversation

@EsadCetiner

Copy link
Copy Markdown
Member

Proposed changes

The pg command isn't installed by default on most Linux distros and is typically not installed afterwards by sysadmins. Fixes a false positive reported in CRS dev chat with the pg command.

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

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

@github-actions

github-actions Bot commented Apr 15, 2026

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

@HackingRepo

HackingRepo commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

do'nt confuse psql with pg @EsadCetiner, so psql must remain blocked

@EsadCetiner

Copy link
Copy Markdown
Member Author

@HackingRepo I'm not sure what your trying to say? I'm removing pg, not psql, what does that have to do with this PR?

@HackingRepo

HackingRepo commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

I see pg is not installed even on Ubuntu, by default and even no APT package exists for it

relunsec@relunsec:~$ pg
pg: command not found
relunsec@relunsec:~$ 

So just we can move the command to PL3 better not just PL2

@HackingRepo

HackingRepo commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

@EsadCetiner

Copy link
Copy Markdown
Member Author

@HackingRepo

I see pg is not installed even on Ubuntu, by default and even no APT package exists for it

That's what I said in my PR description.

So just we can move the command to PL3 better not just PL2

There's no stricter sibling rule at PL-3, and I don't see a reason to even make a PL-3 rule.

@HackingRepo

HackingRepo commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

No, at PL2 that still can cause FPs @EsadCetiner, it must be also excluded at PL1 and PL2 both not just PL1

@HackingRepo

Copy link
Copy Markdown
Contributor

simply add also it to unix-shell-fps-pl2.ra file not just unix-shell-fps-pl1.ra

@EsadCetiner

Copy link
Copy Markdown
Member Author

@HackingRepo

No, at PL2 that still can cause FPs, it must be also excluded at PL1 and PL2 both not just PL1

Ok, and? do you have anything to prove this is a big enough problem at PL-2? As far as I can tell this specific false positive doesn't look too common, and considering the fact pg is not very commonly installed it provides low value at level 1.

simply add also it to unix-shell-fps-pl2.ra file not just unix-shell-fps-pl1.ra

As I mentioned before, there is no stricter sibling rule at PL-3 which can detect pg.

@EsadCetiner EsadCetiner added the backport:lts-4.25.1 PR that must be backported to LTS release 4.25.1 label Apr 16, 2026
@fzipi
fzipi force-pushed the fix-remove-pg-command branch from 9e3ebe4 to a1a77e6 Compare April 18, 2026 13:09
@fzipi

fzipi commented Apr 18, 2026

Copy link
Copy Markdown
Member

I'll say let's first merge this one, and if @HackingRepo wants to run some tests to see the value of moving to major PLs we can reconsider.

@fzipi
fzipi added this pull request to the merge queue Apr 18, 2026
Merged via the queue into coreruleset:main with commit 82195a4 Apr 18, 2026
9 of 10 checks passed
@EsadCetiner
EsadCetiner deleted the fix-remove-pg-command branch April 18, 2026 16:15
@fzipi fzipi mentioned this pull request Jul 1, 2026
fzipi added a commit that referenced this pull request Jul 2, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:lts-4.25.1 PR that must be backported to LTS release 4.25.1 release:fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants