Skip to content
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.

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

False Positive with "Union von" rule id 942190 #2047

Closed
franbuehler opened this issue Mar 29, 2021 · 4 comments · Fixed by #2058
Closed

False Positive with "Union von" rule id 942190 #2047

franbuehler opened this issue Mar 29, 2021 · 4 comments · Fixed by #2058
Assignees

Comments

@franbuehler
Copy link
Contributor

Description

We already fixed false positives with the sql keyword "union". This is a new one:

Reproduce:

curl -X POST -d 'test=Die "Union von Europa"' localhost

Audit Logs / Triggered Rule Numbers

Rule ID 942190:

[2021-03-29 13:28:58.019534] [-:error] 127.0.0.1:60960 YGHWGX8xxxxxxxxxx06YAAACC [client 127.0.0.1] \
ModSecurity: Warning. Pattern match "(?i:(?:[\\"'`](?:;?\\\\s*?(?:having|select|union)\\\\b\\\\s*?[^\\\\s]|\\\\s*?!\\\\s*?[\\"'`\\\\w])|(?:c(?:onnection_id|urrent_user)|database)\\\\s*?\\\\([^\\\\)]*?|u(?:nion(?:[\\\\w(\\\\s]*?select| select @)|ser\\\\s*?\\\\([^\\\\)]*?)|s(?:chema\\\\s*?\\\\([^\\\\)]*?|elect.*?\\\\w?user\\\\()|in ..." \
at ARGS:test. [file "/home/fb/git/coreruleset/rules/REQUEST-942-APPLICATION-ATTACK-SQLI.conf"] [line "200"] [id "942190"] \
[msg "Detects MSSQL code execution and information gathering attempts"] \
[data "Matched Data: \\x22Union v found within ARGS:test: Die \\x22Union von Europa\\x22"] [severity "CRITICAL"] \
[ver "OWASP_CRS/3.3.0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-sqli"] \
[tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/152/248/66"] [tag "PCI/6.5.2"] [hostname "localhost"] [uri "/"] \
[unique_id "YGHWGX8xxxxxxxxxx06YAAACC"]

[x] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.

I can have a look at this, but wanted to document it here so that I don't forget it.

@noneisland
Copy link

Can you share how you fix this false positive?

I submit a similar false positive #2044 but didn't get any response.

I'd like to fix it by myself if possible.

@franbuehler
Copy link
Contributor Author

I will solve the false positive by expanding the regex so that it matches less aggressively.

Using the example of UNION:
UNION alone does not make much sense. It makes sense with UNION ALL, UNION DISTINCT or UNION SELECT.
In this commit (6c5f455) I expanded the keywords UNION and ALTER with "all" valid SQL commands that can follow: 6c5f455#diff-ca2e05f6521d61919bf1d74d07d511cd1ccef6bdf553103cac21c0d757979044R132

This will reduce the number of false positives, but unfortunately we may also introduce poorer coverage and possible false negatives. In this case, in this old commit, we moved the two keywords UNION and ALTER to a strict sibling at paranoia level 2. To still have the coverage at a higher paranoia level.

A fix, a pull request, would be very welcome. Please let us know if we can help.

@dune73
Copy link
Member

dune73 commented Apr 6, 2021

We'll get to #2044 in due time. Sorry for taking so long, but it's a lot of issues...

@dune73
Copy link
Member

dune73 commented Apr 19, 2021

We talked about this in the April issue chat.

Given the PR #2058 is on good tracks, we close this issue in favor of the PR.

@dune73 dune73 closed this as completed Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants