8000 942310 regular suspected error · Issue #2118 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

942310 regular suspected error #2118

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 Jun 7, 2021 · 21 comments
Closed

942310 regular suspected error #2118

xiaoxiaofeiduo opened this issue Jun 7, 2021 · 21 comments
Assignees
@franbuehler @xiaoxiaofeiduo

Comments

@xiaoxiaofeiduo
Copy link
Contributor

Describe the bug

There may be a problem with [\"'`]\s+and\s*?=\W in the 942310 rule. You can view this part in the /util/regexp-assemble/regexp-942310.data file. By analyzing the regularity, There can only be \s between [\"'`] and and, and \w cannot be after and, such as " and =;, but I check related payloads such as: AND 1=1 AND '%'=' Unable to be hit, more related payloads can refer to:
https://github.com/payloadbox/sql-injection-payload-list ,
https://www.netsparker.com/blog/web-security/sql-injection-cheat-sheet/ .
Maybe I didn’t find the corresponding detection payload, let’s take a look at it together~

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:
@franbuehler
Copy link
Contributor

Hi again @NiceYouKnow 👋

Thanks again for your very welcome issue!
We'll investigate it shortly.

@franbuehler franbuehler self-assigned this Jun 7, 2021
@franbuehler
Copy link
Contributor 8000

I searched all source files for and.

The regexes that contain and and detect AND 1=1 AND '%'=' are:

regexp-942400.data:\band\b\s+\d{1,10}\s*?[=<>]
regexp-942400.data:\band\b\s+'[^=]{1,10}'\s*?[=<>]
regexp-942400.data:\band\b\s+\d{1,10}
regexp-942400.data:\band\b\s+'[^=]{1,10}'
regexp-942400.data:\band\b ?\d{1,10} ?[=<>]+
regexp-942400.data:\band\b ?['"][^=]{1,10}['"] ?[=<>]+

Rule 942400 is at PL2.

We have to discuss if we want to extend the detection at PL1 or PL2 is enough...

@xiaoxiaofeiduo
Copy link
Contributor Author

AND 1=1 AND'%'=' is just an example. What I want to express is that

["'`]\s+and\s*?=\W

may have a problem, and I cannot understand the payload it can hit.

@franbuehler
Copy link
Contributor

Ok, I misunderstood your problem. 😀

I also don't know which payload was meant here with this regex:
["']\s+and\s*?=\W`

Some examples that probably don't make sense 😉 But so we can understand the pattern:
https://regex101.com/r/olcs1Q/1

Maybe someone with SQLi knowhow can help us?
@theMiddleBlue can you help us here?

@franbuehler
Copy link
Contributor

Hi @theMiddleBlue

Do you know what payload should be detected with [\"']\s+and\s*?=\W` in rule 942310 (https://github.com/coreruleset/coreruleset/blob/v3.4/dev/util/regexp-assemble/regexp-942310.data#L1) and if this makes sense or if we have a problem with this regex?

I tried to write some examples which probably don't make sense:
https://regex101.com/r/olcs1Q/1

Could you please help us here?

@dune73 dune73 mentioned this issue Jul 19, 2021
Closed
@theMiddleBlue
Copy link
Contributor

Hi @franbuehler sorry for being late. Agree with you, It doesn't make sense to me too. If we're talking about SQL I doubt that and = represents a valid syntax.

@dune73 dune73 mentioned this issue Aug 16, 2021
Closed
@dune73
Copy link
Member
dune73 commented Aug 16, 2021

Could somebody come up with a status update here?

@xiaoxiaofeiduo
Copy link
Contributor Author

My idea is to remove this regularity because it doesn't make sense to me. I don't know what others think. thanks~ ^~^

@franbuehler
Copy link
Contributor

Yes, in my opinion this regex can be removed from the source file and the regex can be regenerated for the rule 942400.
And @theMiddleBlue confirms this too.
Would we like @NiceYouKnow to do a PR here?

@dune73
Copy link
Member
dune73 commented Aug 16, 2021

Thank you @NiceYouKnow.

A PR sounds like a decent plan, yes.

@xiaoxiaofeiduo
Copy link
Contributor Author
xiaoxiaofeiduo commented Aug 16, 2021 •
edited
Loading

@franbuehler, The modified rule should be 942310, not 942400, ^~^. I'll submit PR later.

@dune73
Copy link
Member
dune73 commen 8000 ted Aug 16, 2021

Thank you @NiceYouKnow. I co-assigned the issue to you too.

@theseion
Copy link
Contributor

I don't agree that we should remove it. We don't know what it does and it is different enough from the rest that it appears to be there on purpose (e.g. not just copy pasta). For example, why does the match require at least one space after the quotes? That seems very specific.

Since we don't know what it does we open us up to an attack. Of course, I'm no SQL expert, it's conceivable that the expression contains an error that cripples the rule. I just want to be sure that we know what we are doing here.

@franbuehler
Copy link
Contributor

During our monthly issue chat meeting, we decided to first find out the intention behind this rule before removing it.
Could you please wait with your PR @NiceYouKnow?

@dune73
Copy link
Member
dune73 commented Aug 16, 2021

We talked this through in our issue chat. In the end we are not really sure what this part of the regex is all about, but we are not 100% sure it's benign / an error.

So we concluded we are going to ask @theMiddleBlue to take this to twitter and ask if any of the red teamers out there can come up with an example attacking payload that is being matched by the regex in https://regex101.com/r/olcs1Q/1?

If we do not get any useful feedback, we will drop it and the PR by @NiceYouKnow is welcome.

@theMiddleBlue can you help us here? If yes, please do your tweet and the CRS twitter will RT.

@RedXanadu
8000 Copy link
Member
RedXanadu commented Aug 17, 2021 •
edited
Loading

This is a bit of a guess, but maybe it's helpful…

The oldest comments I could find that are related to this regular expression gave some "Example Payloads Detected" which made use of compound assignment, e.g. aa" =+ - "0

That made me wonder if the =\W part of the regex was designed to catch =+ and =-.
Could it be looking for payloads like ' AND =+ 1?

@theMiddleBlue
Copy link
Contributor

@theMiddleBlue can you help us here? If yes, please do your tweet and the CRS twitter will RT.

https://twitter.com/AndreaTheMiddle/status/1427618583354413076?s=20

@xiaoxiaofeiduo
Copy link
Contributor Author

I looked up the relevant information and found nothing to match.
https://github.com/payloadbox/sql-injection-payload-list
https://www.netsparker.com/blog/web-security/sql-injection-cheat-sheet/

@dune73
Copy link
Member
dune73 commented Aug 17, 2021

Thank you guys. Retweeted with the coreruleset account. We'll see.

@dune73
Copy link
Member
dune73 commented Sep 3, 2021

OK, no responses to the tweet and nothing substantial to confirm the idea the regex is correct. Thank you for your alternative proposal @RedXanadu.

So I guess it's time for your fix / PR @NiceYouKnow. Thank you for your support.

@theseion
Copy link
Contributor
theseion commented Sep 5, 2021

I just spent some time with this too. The longer I look at it the more it seems to me that the expression contains at least one typo. For the expression to match there must be an engine accepting something like AND = which doesn't make sense because there is no left hand side to the =. Unless there existed a "boolean compound assignment" somewhere (e.g. x := y AND= z).

While the idea of compound operators sounded promising at first, I haven't been able to find any operator where the = is the the first of the two characters. In every dialect the = is always the last character of the compound operator. Even if there where such an operator, the observation above still holds: there would be no left hand side to the operator.

In conclusion, I think the odds of opening us up to an attack are slim (even if there were an exploit, it doesn't seem to be widely known) and I think we should remove the expression.

Thanks @NiceYouKnow, these kinds of issues are so hard to find and we really need the support of CRS users to help us improve the quality of the rules.

@xiaoxiaofeiduo xiaoxiaofeiduo mentioned this issue Sep 6, 2021
Merged
@dune73 dune73 mentioned this issue Sep 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@franbuehler franbuehler

@xiaoxiaofeiduo xiaoxiaofeiduo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@theseion @dune73 @theMiddleBlue @RedXanadu @franbuehler @xiaoxiaofeiduo
0