-
-
Notifications
You must be signed in to change notification settings - Fork 402
feat: added detection for quote evasion #3813
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
base: main
Are you sure you want to change the base?
Conversation
This should already be handled by the existing rules. The existing rules require a prefix to reduce false positives, e.g.,
|
@Xhoenix ☝️ |
This is a bypass without the need of a prefix, something like 932230 at PL 1, i.e Direct command injection. |
This is probably very prone to false positives al PL1, right? |
I mean, we are very careful trying to get only some words (that's why we have lists of words). The regular expression you created there it is only going to give problems for a generic match. So we need to do better or everyone will disable this rule. 🤷 |
Yes, I was thinking PL2 would be more appropriate, as some other common bypasses are at PL2. So, should I move this to PL2? |
@Xhoenix I think this needs to be part of a more general discussion. Currently, we avoid generic matching rules for RCE on purpose, to prevent FPs. We have these rules for all PL levels, so the example given above will also work at PL2 for the list of words we check for at that level. Even with the prefix checks we experience high numbers of FPs for some strings, so if we wanted to start going prefix-less, that would have to be a group discussion, best based on metrics. If you can prove that this rule will not create more FPs (which I doubt you can because we don't even know the rate for the current rules), then we could certainly adopt it. |
I was thinking about rule 932240 at PL2, which I think is way more prone to FPs(e.g |
Common sense is not tests. Tests are proofs. Let's see what we can do, but without numbers it is very difficult to make an informed decision... |
Looking fwd to tonight’s meeting. 🙂 |
Do you have numbers for the discussion? Otherwise, how are you going to support it? |
I think this PR is perfect for PL 2, without any modifications. Expecting people to make typos like "no space after quotes" is just not practical in the sense the rule is at PL 2 and targets ARGS. So, LGTM. |
QQ: what do we think ARGS has? Because I see possible a lot of text coming to that variable, and that is how you get your FPs. |
Then, as per what I wrote above, rule 932240 shouldn't exist either. It's atleast twice as probable to cause FPs than this simple rule. You can check the regex-assembly file 932240, which lists the payloads in details. |
Co-authored-by: azurit <jozef@sudolsky.sk>
Sadly we didn't had any response yet on this one. I'll try to include more people next. |
📊 Quantitative test results for language: |
Is there any updates on the test? Some positive results will get this PR merged. 🙂 |
We're talking to them. But communication is always slow in these instances. |
Are we there yet? 😂 |
Added detection for shell quote evasion.