Skip to content
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

False positive caused by one character matches in LDAP regex #47

Open
dnet opened this issue Apr 23, 2018 · 4 comments
Open

False positive caused by one character matches in LDAP regex #47

dnet opened this issue Apr 23, 2018 · 4 comments

Comments

@dnet
Copy link

dnet commented Apr 23, 2018

The LDAP regex looks like this

((dn|dc|cn|ou|uid|o|c)=[\w\d]*,\s?){2,}

However, we've found minified JS code that happened to use o and c right next to each other in a variable declaration, thus they were delimited by commas.

var r=this.doFooBar(n,i[t]),o=r,c=null,l=[],a=i[t];
//                          ^^^^^^^^^^^---------------- this matched

What would be a good fix? Would removing o and c (but leaving the five others) really harm LDAP exception detection that much?

@augustd
Copy link
Owner

augustd commented Apr 23, 2018

It is a tough call. The regex is already pretty specific, so making changes could create a false negative situation.

It seems like the issues you have reported so far (this and #45) have come on .js files. Maybe we should really be looking at a way to specify the types of files (either MIME type or extension) that should be scanned, or lowering the confidence of results when they appear on what should be a static file.

@dnet
Copy link
Author

dnet commented Apr 24, 2018

That's a great idea, methods getInferredMimeType() and getStatedMimeType() of the class IResponseInfo could be used for getting the MIME type of the response. Sometimes these JS monsters are generated dynamically, so file extension is non-existent in the request, however, it could still be useful, method getURL of the class IRequestInfo is a good way to get that. Maybe with a blacklist approach (like ignore this rule if MIME type or extension suggests JS) we could minimize false negatives.

What do you think? Should I implement this and submit a pull request, or do you prefer to tackle it?

@augustd
Copy link
Owner

augustd commented Apr 24, 2018

I'd prefer to see the confidence rating bumped down by one (e.g. Firm -> Tentative) rather than having the rule be completely skipped. As you say, sometimes these files are generated dynamically, and that could throw errors.

It will be a while before I have time to do this so by all means send a pull request!

@dnet
Copy link
Author

dnet commented Apr 24, 2018

I'd prefer to see the confidence rating bumped down by one (e.g. Firm -> Tentative) rather than having the rule be completely skipped.

I just had the same thought ~15 minutes after posting my reply. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants