Shell false positives for rules 932260 and 932236 #3631
Comments
|
Thank you very much for an excellent bug report. And sorry for the hassles. You are not on the full CRS 4.0.0 release, but I've looked up the 4 examples they gave and they are still set as reported by you. So just make sure we're on the same page, you suggest the following: ./regex-assembly/include/unix-shell-upto3.ra
./regex-assembly/include/unix-shell-4andup.ra
But this is part of a larger problem for you. You happen to use Most of the commands in unix-shell-upto3.ra come with the This is substantial feedback and while the suggestions above are easy to implement, the general extension of all the commands in unix-shell-upto3.ra takes some thinking. |
|
I don't have any suggestions for fd and grc at this time since I'm not really sure what they were originally intended to match on. These could be exact matches or partial prefixes for a whole suites of utilities behind it, covering things like fdisk, fd(u)mount, fdfind, etc. grc could be the generic colouriser at https://github.com/garabik/grc. If so, and this pattern is necessary, then it may be worth splitting those up into grc@ and grcat. I tried to find changelogs and checkins for these but didn't notice anything. If 'df' is meant to match on the actual df command then changing it to 'df@' may be prudent. These are all PL2, meaning increased scrutiny and potentially increased false positives as well. I think most people understand that and that's fine. I don't want to presume anything or recommend changes that would drastically weaken the higher paranoia levels. I've already solved my issue with the grcxyz_ cookie prefix issue, and can make other changes to reduce the PL2 usage for suspicious traffic. I just thought this would be worth mentioning given what I've been seeing and the potential for 'df' and 'fd' to match on hex or UUID type values that may be on cookies or parameters and their related usage in the Referer header. sudo is a PL1 so I do think that's worth considering. On Debian 12 the sudo package has 5 binaries that start with 'sudo'. If it was necessary to match on most or all of them then 'sudo' could be broken out into: sudo@, sudoedit, sudoreplay, sudo_logsrvd, and sudo_sendlog. sudoedit is a symlink to sudo. The longer binary names seem far less likely to hit on random strings like the SUDoXyZ... fpestid cookie I ran across the other day so they may not need the @. Who knows, maybe it's already very unlikely for someone to run across a cookie or other random value that starts with sudo. But generally speaking if we know or can make an educated guess on what the original intent was then maybe it's worth being more precise with some of them, especially the shorter strings. |
|
I've taken a crack at trying to solve this issue, and I don't think adding The real problem here is that invalid commands are being matched when neither Either way, it will take some time to find all the commands that each entry in the unix-shell-* was intended to match, and some commands will likely be missed (Simply due to human error) so a false negative may be unavoidable. |
|
I am not very familiar with CRS, but wouldn't it be enough to check whether there is at least one alphabetic or numeric characters before or after the match and if this is the case than this can not be a valid unix command so reject it. There seems to be an equivalent discussion about the php functions, which sounds very similar: #3273 (comment) |
|
Thanks @EsadCetiner for trying. Indeed, this is only one of several reports of issues with the shell injection rules and we need to solve this on a conceptual level and not case by case. @niklasweimann That's not a bad idea. @EsadCetiner do you want to give it a try? I currently have a few other things to finish. |
theseion
added
the
v4 unix rce
|
@niklasweimann Sorry for the late reply
we're already excluding commands at PL-1 that match with common english words, but thank you for the suggestion. @theseion However I have to add this to end of all commands, and I don't know how bad the performance will be since I'm using a non capturing group. Although in the past 3 weeks I did find all of the commands with 5 characters or less that don't require arguments and added |
|
Thanks @EsadCetiner. I'll get back to the RCE issue once I'm done with a couple of other things. If anybody else want's to take a crack at the Unix RCE rules in the mean time, I'll be happy to give some pointers. |
|
If nobody else has any other ideas, I'm going to open a PR that prevents invalid matches on commands with 5 characters or less. I'm not sure how ideal this solution is, but it does work. |
|
Go for it, I think it's a good idea. Be careful though, This is probably not what you want. Your solution needs to take the use of these suffixes into account. Maybe they can be dropped, be applied conditionally, .... Ping me if you have questions. |
|
@theseion wdym by conditionals, I thought they weren't allowed because of ReDoS risk? |
|
Ah 😄. I wasn't talking about regex conditionals. I mean conditionally in the sense that the crs-toolchain could, theoretically, be used to append a suffix conditionally. The condition would then be part of the regex-assembly macro language. |
|
@theseion I know I can use a suffix marker |
|
Well, as I've said elsewhere, I think we need to solve this with an altered approach, which will probably require some changes to the setup of the way the regular expressions are generated for the 932xxx rules, and / or changes to the crs-toolchain. For example, we could extend the Then the crs-toolchain could generate the regular expression for some command There are two things to consider here:
There are probably other ways to go about this, I'm just trying to give you some idea where this could go. I know this might all seem a bit much, I'll try to put more energy into this soon. |
I thought about this too, but I'm not sure if there's a way to specify a default suffix (i.e when neither @ or ~ is specified, append I don't know if it's a good idea to get rid of |
|
I think you need the semantics of |
Description
I've encountered some shell false positives for 932260 (PL1) , 932236 and 932239 (PL2) for commands like sudo, df, fd, and grc.
Some of these I'm obviously familiar with, but others like 'fd' or 'grc' I've never come across before. I'm not sure exactly what they are or how commonly they may be used in attacks/information leakage, or how relaxed is even appropriate. Would it be acceptable for 'sudo' to be broken out into sudo@ and sudoedit, sudoreplay, etc, fd to fd@, df to df@. I'd think this would be ok, but I also don't know if there are additional commands that these substrings were intended to match on. And more generally should these lists be revisited to ensure the matches are concise enough? I feel like there's some potential for additional FPs on unix-shell-upto3.ra and even some of the 4 character words on unix-shell-4andup.ra (expr, sched, uniq).
I didn't include any examples for grc. It just happens to match on the first 3 characters of a cookie prefix we use for a site and I've taken care of those individually. Just thought it was worth mentioning due to my unfamiliarity and the general questions.
How to reproduce the misbehavior (-> curl call)
932260 - PL1 on 'sudo' in REQUEST_COOKIES:fpestid
932236 - PL2 on 'df', 'fd', in ARGS (uniqid, uuid, or hex type values)
and REQUEST_COOKIES (ids, hex values, etc)
932239 - PL2 for REQUEST_HEADERS:Referer
Your Environment
Confirmation
[X] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.
The text was updated successfully, but these errors were encountered: