-
-
Notifications
You must be signed in to change notification settings - Fork 401
feat: added rule to detect Bash Brace Expansion #3780
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
Do we need a completely new rule for this? |
Any suggestions on a rule where this might fit into are welcome. |
Might be something to discuss in the next meeting. I'll add an item to the list. |
Any suggestions on a rule where this might fit into are welcome. I created a new rule to keep things simple, but willing to adapt if decided to. |
I just realised that we already try to detect brace expansion in combination with shell RCE: coreruleset/regex-assembly/toolchain.yaml Line 17 in 38bef47
|
But the payloads I mentioned are bypassing CRS, maybe because there isn't a rule for it? |
Well, the pattern is only detected when the |
The patterns for the command li 8000 ne processor are defined here: https://github.com/coreruleset/coreruleset/blob/main/regex-assembly/toolchain.yaml. |
Only commands that are part of the commands list and are not filtered due to FPs for a given rule will be detected.
We probably also need to extend 932240 to detect brace expansion as part of the generic detection. Well, we would need to make a couple of changes in any case, at different locations. While it would be consistent, I'm leaning towards a single rule (maybe two at different PLs). The complexity will not do us any good. |
Should I add the |
Can you add an example on how to use the |
There are only a few commands that use it, e.g. |
@Xhoenix Can you add an example on how to use the +? |
From bash manpage:
Alias in zsh also uses the +. |
Thanks for the explanation. I don't see the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I see here is that both @azurit and @theseion commented about their concern on adding a new rule. I share that same concern.
If we should capture this in rule 932240 I think it is best to focus on that case.
Can you follow on Max's findings here #3780 (comment) and here #3780 (comment), and update and test the relevant files?
@fzipi As I wrote in #3780 (comment), I'm leaning more towards a new rule now, because we'd need to make changes in multiple places to get this working with the current RCE rules. Also, the rules are already too complex and adding another detection will only make it harder to maintain this particular detection. |
📊 Quantitative test results for language: |
LGTM! |
Ping! |
Bash Brace Expansion:
{,ip,a}
{l,-lh}s
{,ifconfig}
etc.I think there is still room for improvement on this rule as there are other things to cover as per Bash/Zsh manual. Any ideas are welcome.