8000 PL 1 False Positive on Blacklist Keywords from Node-Validator · Issue #2060 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

PL 1 False Positive on Blacklist Keywords from Node-Validator #2060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
1 task done
RubieV opened this issue Apr 28, 2021 · 3 comments
Closed
1 task done

PL 1 False Positive on Blacklist Keywords from Node-Validator #2060

RubieV opened this issue Apr 28, 2021 · 3 comments
Labels
➕ False Positive 🥇 good first issue Good for newcomers

Comments

@RubieV
Copy link
RubieV commented Apr 28, 2021

Description

For several environments, the PL1 rule Blacklist Keywords from Node-Validator (941180) , is triggering on legitimate user's input, especially in customer feedback forms.

The rule itself contains good signatures for the detection of actual attacks (i.e. document.cookie), however is mixed with the payload -->, which is causing the common false positives.

Audit Logs / Triggered Rule Numbers

Example

Look at this example --> https://example.com/
customer requested new e-mail adres  --­> john@example.com

Substantiation

Signature False Positive Ratio
document.cookie Low
document.write Low
.parentnode Low
.innerhtml Low
window.location Low
-moz-binding Low
<!-- Low
--> High
<![cdata[ Low

Proposal

Splitting this rule from a single PL1 rule into a PL1 and an PL2 rule.

Your Environment

  • CRS version (e.g., v3.2.0): v3.2.0
  • Paranoia level setting: 2
  • ModSecurity version (e.g., 2.9.3): 2.9.3
  • Web Server and version (e.g., apache 2.4.41): Multiple
  • Operating System and version: Multiple

Original Rule

SecRule REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|ARGS_NAMES|ARGS|XML:/* "@pm document.cookie document.write .parentnode .innerhtml window.location -moz-binding <!-- -->
 <![cdata[" \
    "id:941180,\
    phase:2,\
    block,\
    capture,\
    t:none,t:utf8toUnicode,t:urlDecodeUni,t:htmlEntityDecode,t:jsDecode,t:cssDecode,t:lowercase,t:removeNulls,\
    msg:'Node-Validator Blacklist Keywords',\
    logdata:'Matched Data: %{TX.0} found within %{MATCHED_VAR_NAME}: %{MATCHED_VAR}',\
    tag:'application-multi',\
    tag:'language-multi',\
    tag:'platform-multi',\
    tag:'attack-xss',\
    tag:'paranoia-level/1',\
    tag:'OWASP_CRS',\
    tag:'capec/1000/152/242',\
    ctl:auditLogParts=+E,\
    ver:'OWASP_CRS/3.3.0',\
    severity:'CRITICAL',\
    setvar:'tx.xss_score=+%{tx.critical_anomaly_score}',\
    setvar:'tx.anomaly_score_pl1=+%{tx.critical_anomaly_score}'"

Confirmation

  • I have removed any personal data (email addresses, IP addresses, passwords, domain names) from any logs posted.
@airween
Copy link
Contributor
airween commented Apr 28, 2021

Hi @RubieV,

thanks for your report. We will investigate this issue shortly, and will give a solution.

@dune73 dune73 mentioned this issue May 17, 2021
Closed
@lifeforms
Copy link
Member
lifeforms commented May 17, 2021 •
edited
Loading

I remember some heated conversations about --> with you @RubieV and yes finally I can agree that we move this pattern to Paranoia Level 2. Thanks for the great report.

This is a great first issue as it's basically copying the existing rule into the Paranoia Level 2 section of the file (remembering to give it a new ruleId and giving it the paranoia-level/2 tag) and moving the --> to that new rule, so we will assign it to @53cur3M3, a new face on the CoreRuleSet scene! And hope to raise a proud new contributor. :)

@lifeforms lifeforms added the 🥇 good first issue Good for newcomers label May 17, 2021
@53cur3M3 53cur3M3 mentioned this issue May 18, 2021
Closed
@dune73
Copy link
Member
dune73 commented May 18, 2021

Closing this in favor of PR at #2082.

Thank you for reporting @RubieV.

@53cur3M3 53cur3M3 mentioned this issue May 19, 2021
Merged
fzipi added a commit that referenced this issue May 20, 2021
@fzipi
Merge pull request #2088 from 53cur3M3/issue2060tests
18e9b7e 
Moved --> keyword from 941180 (PL1) into new stricter sibling rule 941181 (PL2) #2060
@dune73 dune73 mentioned this issue Jun 3, 2021
Closed
@dune73 dune73 closed this as completed Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
➕ False Positive 🥇 good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@airween @dune73 @lifeforms @RubieV
0