8000 False positive on parameter value XMLNS · Issue #2071 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

False positive on parameter value XMLNS #2071

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
noneisland opened this issue May 10, 2021 · 14 comments
Closed

False positive on parameter value XMLNS #2071

noneisland opened this issue May 10, 2021 · 14 comments
Assignees
@lifeforms
Labels
➕ False Positive
Milestone
CRS v3.4.0

Comments

@noneisland
Copy link

Description

GET /api/v1/query?q=7XMLNS triggered false positive because the parameter contains "XMLNS"

#16 4.565 Rule Id: 941130 phase: 2
#16 4.565 * Match, but no disruptive action: ModSecurity: Warning. Matched "Operator Rx' with parameter (?i)\s\S\b' against variable ARGS:q' (Value: 7XMLNS' ) [file "/opt/coreruleset/rules/REQUEST-941-APPLICATION-ATTACK-XSS.conf"] [line "125"] [id "941130"] [rev ""] [msg "XSS Filter - Category 3: Attribute Vector"] [data "Matched Data: 7XMLNS found within ARGS:q: 7XMLNS"] [severity "2"] [ver "OWASP_CRS/3.3.0"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-xss"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/152/242"] [hostname ""] [uri "/api/v1/query"] [unique_id "162043736477.530879"] [ref "o0,6v20,6t:utf8toUnicode,t:urlDecodeUni,t:htmlEntityDecode,t:jsDecode,t:cssDecode,t:removeNulls"]

Your Environment

CRS version: default v3.4/dev
Paranoia level setting:
ModSecurity version : 3.0.4
Web Server and version :
Operating System and version: Amazon Linux 2

Confirmation

[ x] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.
@noneisland noneisland changed the title False positive on value XMLNS False positive on parameter value XMLNS May 10, 2021
@lifeforms
Copy link
Member
lifeforms commented May 10, 2021 •
edited
Loading

Hi @noneisland, thanks for your report.

Your parameter is triggered by the pattern (?i)[\s\S]xmlns\b in regexp-941130.data.

It looks to me, as if we could safely get rid of the false positive by ensuring a word boundary, as in: (?i)[\s\S]\bxmlns\b.

We could do the same for some other words in this file, I would propose to change it to:

(?i)[\s\S]\bxlink:href\b
(?i)[\s\S]\bxhtml\b
(?i)[\s\S]\bxmlns\b
(?i)[\s\S]!ENTITY\s+(?:\S+|%\s+\S+)\s+SYSTEM\b
(?i)[\s\S]!ENTITY\s+(?:\S+|%\s+\S+)\s+PUBLIC\b
(?i)[\s\S]\bdata:text/html\b
(?i)[\s\S]\bformaction\b
(?i)[\s\S]@import\b
(?i)[\s\S];base64\b
(?i)[\s\S]pattern\b.*?=\b

The rule is an XSS rule. Since I don't have so much experience with XSS myself, I will discuss this with our dev team on our next meeting before proceeding, as I don't want to introduce vulnerabilities.

Again thanks for the report and we will follow up soon.

@lifeforms lifeforms self-assigned this May 10, 2021
@lifeforms lifeforms added this to the CRS v3.4.0 milestone May 10, 2021
@noneisland
Copy link
Author

Hi @lifeforms , thank you for your quick response.

I'm interested in contributing to fix the false positive if somebody can tell me how we build and test the changes to the coreruleset.

I discovered these false positives when using coreruleset with my application logs.

If somebody can give me some guidance, I probably will be able to fix these false positives quicker because they are a blocker for my use case.

@lifeforms
Copy link
Member
lifeforms commented May 10, 2021 •
edited
Loading

Normally you can just change the rule files in place, but in this case, it's a bit more complicated because rule 941130 is compressed from a series of regular expressions into one big regular expression. This helps us keep the number of rules lower and make processing faster while keeping related patterns in one rule.

The source regexps can be found in the file regexp-941130.data. These are the ones that could be improved, using my example for instance.

But this file is not directly used by web servers. We have to insert the compressed version into the *.conf rule file first.

When you look at the comment on rule 941130 you find that we run a perl command to generate the compressed regexp, and the output of that perl script is the one that we put in the rule file.

To compile the regexps into one, you need Perl and the Regexp::Assemble CPAN module. You can read more about it in our blog on Regexp::Assemble.

With the updated .data file and the changed rule file, it is almost ready for a PR. The final step for a perfect PR is to add a test case for your false positive to our regression tests. This ensures that you definitely have fixed the problem and it protects us from reintroducing the error again in a future change. To do this, you add a section at the bottom of the test file 941130.yaml that contains your payload, but it should end with no_log_contains: id "941130" because, if the new rule is correct, it should NOT trigger rule 941130 any longer.

Good luck!

@noneisland
Copy link
Author

hi @lifeforms , thanks for the information.

I am wondering if this can be made easier, e.g., "make testruleset" will automatically tell you if your changes succeeds or fails.

@lifeforms
Copy link
Member
lifeforms commented May 10, 2021 •
edited
Loading

It's possible to run the tests with Docker but my setup is currently not working anymore so I hope someone else can pitch in on how to run the tests locally, @franbuehler or @fzipi ?

@fzipi
Copy link
Member
fzipi commented May 11, 2021

Sure, it is possible.

You need python3, docker, and docker-compose installed.

The steps should be:

pip install --upgrade setuptools wheel
pip install -r tests/regression/requirements.txt
mkdir -p tests/logs/modsec2-apache/apache2
docker-compose -f ./tests/docker-compose.yml up -d modsec2-apache
py.test -vs --tb=short tests/regression/CRS_Tests.py \
  --config="modsec2-apache" \
  --ruledir=./tests/regression/tests

Will write a wiki page with this.

@noneisland
Copy link
Author

@fzipi , thanks for the instructions. I followed the guide and set it up. But in the last step, it doesn't appear that any tests were actually ran.

py.test -vs --tb=short tests/regression/CRS_Tests.py --config="modsec2-apache" --ruledir=./tests/regression/tests
======================================================================================================= test session starts ========================================================================================================
platform darwin -- Python 3.7.3, pytest-4.6.0, py-1.10.0, pluggy-0.13.1 -- /code/coreruleset/bin/python3
cachedir: .pytest_cache
rootdir: /code/coreruleset
plugins: ftw-1.2.4
collected 1 item

tests/regression/CRS_Tests.py::test_crs[ruleset0-test0] SKIPPED

==================================================================================================== 1 skipped in 0.01 seconds =================

@noneisland
Copy link
Author

Hi @noneisland, thanks for your report.

Your parameter is triggered by the pattern (?i)[\s\S]xmlns\b in regexp-941130.data.

It looks to me, as if we could safely get rid of the false positive by ensuring a word boundary, as in: (?i)[\s\S]\bxmlns\b.

We could do the same for some other words in this file, I would propose to change it to:

(?i)[\s\S]\bxlink:href\b
(?i)[\s\S]\bxhtml\b
(?i)[\s\S]\bxmlns\b
(?i)[\s\S]!ENTITY\s+(?:\S+|%\s+\S+)\s+SYSTEM\b
(?i)[\s\S]!ENTITY\s+(?:\S+|%\s+\S+)\s+PUBLIC\b
(?i)[\s\S]\bdata:text/html\b
(?i)[\s\S]\bformaction\b
(?i)[\s\S]@import\b
(?i)[\s\S];base64\b
(?i)[\s\S]pattern\b.*?=\b

The rule is an XSS rule. Since I don't have so much experience with XSS myself, I will discuss this with our dev team on our next meeting before proceeding, as I don't want to introduce vulnerabilities.

Again thanks for the report and we will follow up soon.

I did some research. "xmlns" itself is not a XSS attack, it has to be combined with javascript in order to be an actual attack.
https://portswigger.net/web-security/cross-site-scripting/cheat-sheet

@dune73 dune73 mentioned this issue May 17, 2021
Closed
@lifeforms lifeforms mentioned this issue May 17, 2021
Closed
@dune73
Copy link
Member
dune73 commented Jun 21, 2021

I'm losing the overview here a bit. Can you guys tell me what the status is?

@dune73 dune73 mentioned this issue Jun 21, 2021
Closed
@lifeforms
Copy link
Member

Meeting decision: I will help the submitter by making the necessary changes myself.
8000

@dune73
Copy link
Member
dune73 commented Jun 23, 2021

For the record: We talked about this issue at our recent project meeting.

@dune73 dune73 mentioned this issue Jul 19, 2021
Closed
@dune73
Copy link
Member
dune73 commented Jul 19, 2021

I know you are very busy @lifeforms, but can you still give us a status?

@dune73
Copy link
Member
dune73 commented Sep 3, 2021

Ping @lifeforms

@lifeforms
Copy link
Member

Addressed this in PR #2192. I'd like to keep the changes as small as possible to move forward on this, we can always perfect the rule later. For further discussion please visit #2192.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lifeforms lifeforms

Labels
➕ False Positive
Projects
None yet
Milestone
CRS v3.4.0
Development

No branches or pull requests
4 participants
@dune73 @fzipi @lifeforms @noneisland
0