Skip to content

Conversation

@touchweb-vincent
Copy link
Contributor

@touchweb-vincent touchweb-vincent commented Sep 5, 2025

Hello,

Just a quick fix on 933153

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2025

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

Copy link
Contributor

@franbuehler franbuehler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for proposing this PR. Well spotted! The rule 933153 is indeed missing the inbound_anomaly_score increase!
The pipeline is failing. Could you please change the PR title, see the error: https://github.com/coreruleset/coreruleset/actions/runs/17489485816/job/49675785714?pr=4260.

@franbuehler
Copy link
Contributor

The missing increase of inbound_anomaly_score is maybe something we could detect with linting? What do you think @theseion or @airween?

@touchweb-vincent touchweb-vincent changed the title Update REQUEST-933-APPLICATION-ATTACK-PHP.conf fix(933153): missing inbound_anomaly_score Sep 5, 2025
@touchweb-vincent
Copy link
Contributor Author

Hello @franbuehler

done for the title

@franbuehler franbuehler self-requested a review September 5, 2025 10:28
Copy link
Contributor

@franbuehler franbuehler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR! LGTM.

@theseion
Copy link
Contributor

theseion commented Sep 5, 2025

The missing increase of inbound_anomaly_score is maybe something we could detect with linting? What do you think @theseion or @airween?

Yes, absolutely.

@touchweb-vincent could you also update rule 933135 and replace rce_score with php_injection_score?

Copy link
Contributor

@theseion theseion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify 933135

@airween
Copy link
Contributor

airween commented Sep 5, 2025

The missing increase of inbound_anomaly_score is maybe something we could detect with linting? What do you think @theseion or @airween?

Absolutely - but at first glance I'm afraid that's not trivial. We should figure out what rules must contain a setvar action with any inbound_anomaly_score variable, how can we describe them with a logical expression. Eg. all rules which contain tag with paranoia-level/N?

@theseion
Copy link
Contributor

theseion commented Sep 5, 2025

Inbound / outbound could be tricky. But we could simply check that every rule with a paranoia level is setting a corresponding anomaly score variable.

@touchweb-vincent
Copy link
Contributor Author

The missing increase of inbound_anomaly_score is maybe something we could detect with linting? What do you think @theseion or @airween?

Yes, absolutely.

@touchweb-vincent could you also update rule 933135 and replace rce_score with php_injection_score?

Hello, done https://github.com/coreruleset/coreruleset/pull/4262/files

@franbuehler franbuehler requested a review from theseion September 5, 2025 13:04
@fzipi fzipi dismissed theseion’s stale review September 5, 2025 21:36

Addressed in new PR.

@fzipi fzipi added this pull request to the merge queue Sep 5, 2025
Merged via the queue into coreruleset:main with commit 8edc58f Sep 5, 2025
7 checks passed
@fzipi fzipi mentioned this pull request Oct 2, 2025
@touchweb-vincent touchweb-vincent deleted the patch-1 branch October 18, 2025 10:11
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.

5 participants