-
Notifications
You must be signed in to change notification settings - Fork 728
LDAP Injection Rule #276
Comments
@dnkolegov : Let's try and get this sorted out. Do you have the capacity to test any new rules proposed here? My ldap knowledge is fairly limited and I do not want to introduce false positives here. I think together we can get it done. |
I propose to consider the following regex:
Tests:
|
Thank you. That looks remarkable. So this is a list of attack strings and a regex, which detects them? What about false positives? |
I tested this regex only for some valid LDAP filters:
|
@dune73 i guess this never got added - it is great work though I think we should put it on the 3.1 list. |
I like this as well. But I think we need somebody with real ldap experience telling us, if FPs are likely. |
Updated regex (fixed whitespaces):
|
This is still not added and given our lack of knowledge for everything LDAP we would rely on @dnkolegov's word to go and include this into our rule set. So @dnkolegov, are you actively using this rule on your server(s)? Any feedback with regards to false positives? |
This rule was developed and used within a proprietary Positive Technologies' Application Firewall. I do not remember customer requests regarding this rule. But I have not been working there for 2 years, so I do not have actual information. |
Thank you for writing in again, @dnkolegov! So what do we do guys: I see this as a new paranoia level 3 rule, or we drop the endeavour despite the good contribution by @dnkolegov because we simply lack the knowledge to support this. |
I would argue to bring back the rule in the default set (paranoia level 1). Vulnerability to LDAP injection may be somewhat rare, but nevertheless it's a well published attack class, I could see it being important to enterprise users, and even OWASP and Webappsec wikis have extensive pages on it. The LDAP injection rule was present in CRS 2.x but removed, I have no data why, but I can't recall ever having operations trouble with the rule myself. Our old rule was apparently not having enough coverage, but with the improvements by @dnkolegov A risk could be false positives, but we'll have our regular development cycle with the release candidates to flesh them out. We can be a little more bold at the start of a cycle, and if we run into trouble we can always put the rule in a higher paranoia level. |
OK. Let's do that. I'll prepare a PR. |
I've done the following rule now
Triggered with Question: Where do we put this. I've added it to 921, protocol attacks, but so far there are only http protocol attacks in this group. Other options? |
During the monthly CRS community chat, I re-confirmed my lifelong commitment to this problem and I promised to make this into a PR and get it merged. Meeting minutes: #1671 (comment) |
New ldap injection rule 921200 (fixes issue #276)
A long story comes to an end. We merged the fix for this with PR #1707. Thank you for your contribution @dnkolegov. |
The following LDAP injection vectors from Alonso-Parada research are not detected by current LDAP Injection Rule:
foo)(sn=100
printer)(uid=*)
printer)(department=fa*)
printer)(department=*fa*)
Burp Suite uses the following vectors to test an LDAP Injection and they are also not detected:
eb9adbd87d)(sn=*
eb9adbd87d)!(sn=*
*)(sn=*
*)!(sn=*
Also it is not obvious the purpose of the top and middle parts of regular expression that check values beginning with
(
:Which LDAP injection context is supposed here?
The text was updated successfully, but these errors were encountered: