8000 False negative in 942230 · Issue #2230 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

False negative in 942230 #2230

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
xiaoxiaofeiduo opened this issue Oct 15, 2021 · 6 comments · Fixed by #2348
Closed

False negative in 942230 #2230

xiaoxiaofeiduo opened this issue Oct 15, 2021 · 6 comments · Fixed by #2348
Assignees
@theseion
Labels
➖ False Negative - Evasion

Comments

@xiaoxiaofeiduo
Copy link
Contributor

Describe the bug

This is the regularity of 942230:

(?i:[\s()]case\s+when.*?then|\)\s*?like\s*?\(|select.*?having\s*?[^\s]+\s*?[^\w\s]|if\s?\([\d\w]\s*?[=<>~])

It can be split into four independent rules:

1: (?i:[\s()]case\s+when.*?then)
2: (?i:\)\s*?like\s*?\()
3: (?i:select.*?having\s*?[^\s]+\s*?[^\w\s])
4: (?i:if\s?\([\d\w]\s*?[=<>~])

My question is that there may be a risk of missing the attack on the fourth rule.

Below are two payloads of the same type:

1: if(1=2, -1, 2) union select table_name from information_schema.tables limit 1
2: IF(7423=7424) SELECT 7423 ELSE DROP FUNCTION xcjl--

You can check the test in https://regex101.com/r/h9Vz4I/2, the regular rule only hits the first payload, but not the second payload.

So my suggestion for modification is：

(?i:if\s?\([\w]+\s*?[=<>~])

Remove \d, because \w already contains it.

The modification effect can be viewed https://regex101.com/r/k0G4Ux/1/

Steps to reproduce

Expected behaviour

Actual behaviour

Additional context

Your Environment

CRS version v3.4/dev

  • CRS version (e.g., v3.2.0):
  • Paranoia level setting:
  • ModSecurity version (e.g., 2.9.3):
  • Web Server and version (e.g., apache 2.4.41):
  • Operating System and version:
@theMiddleBlue
Copy link
Contributor

Good catch @NiceYouKnow thanks!
is IF(1=1) SELECT 1 ELSE SELECT 2; a valid SQL syntax? Testing it on MySQL I got:

mysql> IF(1=1) SELECT 1 ELSE SELECT 2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IF(1=1) SELECT 1 ELSE SELECT 2' at line 1

maybe I'm missing something

@lifeforms
Copy link
Member
lifeforms commented Oct 15, 2021 •
edited
Loading

I am just peering through this issue, but in MySQL, the syntax for the CASE operator is different. The example would be correct as:

mysql> select case when 1=1 then 1 else 2 end;
+---------------------------------+
| case when 1=1 then 1 else 2 end |
+---------------------------------+
|                               1 |
+---------------------------------+

However, I don't know how other SQL dialects allow this. What servers accept your notation?

@dune73
Copy link
Member
dune73 commented Nov 15, 2021

@NiceYouKnow are you still interested in this? Could you respond to @lifeforms' question?

@xiaoxiaofeiduo
Copy link
Contributor Author

I am just peering through this issue, but in MySQL, the syntax for the CASE operator is different. The example would be correct as:

mysql> select case when 1=1 then 1 else 2 end;
+---------------------------------+
| case when 1=1 then 1 else 2 end |
+---------------------------------+
|                               1 |
+---------------------------------+

However, I don't know how other SQL dialects allow this. What servers accept your notation?

Your example is correct, no problem. But my question is about (?i:if\s?\([\d\w]\s*?[=<>~]).

@fzipi fzipi mentioned this issue Dec 6, 2021
Closed
@theseion theseion self-assigned this Dec 20, 2021
@fzipi fzipi mentioned this issue Jan 2, 2022
Closed
@theseion theseion changed the title 942230 regular suspected error False negative in 942230 Jan 5, 2022
@theseion theseion mentioned this issue Jan 5, 2022
Merged
@theseion
Copy link
Contributor
theseion commented Jan 5, 2022

Nice catch @NiceYouKnow. The exact query syntax isn't relevant. The issue is that by the rule could be circumvented by using a number > 9 in the comparison.

I've moved the four statements into a new regex data file.

I also adjusted the fourth rule to fix the false negative (test included) and removed the unnecessary character class still left in the original suggestion ([\w]).

@dune73
Copy link
Member
dune73 commented Jan 17, 2022

Closing this in favor of #2348. Thank you for reporting @NiceYouKnow and for providing a PR @theseion.

@dune73 dune73 closed this as completed Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@theseion theseion

Labels
➖ False Negative - Evasion
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed false negative for 942230 theseion/coreruleset
5 participants
@theseion @dune73 @lifeforms @theMiddleBlue @xiaoxiaofeiduo
0