Java LDAP Injection (CWE-90)

[ggolawski]

This PR adds a query to detect LDAP Injections in Java. Plain Java JNDI, UnboundID, Spring LDAP and Apache LDAP API are covered.

This query detects the following LDAP Injection variants:

JNDI

• DirContext.search with tainted DN or filter, for example:

  ctx.search(taintedDN, taintedFilter, new SearchControls());

• DirContext.search with tainted LdapName, for example:

  ctx.search(new LdapName(taintedDN), filter, new SearchControls());

  or

  ctx.search(new LdapName("ou=people").addAll(new LdapName(taintedDN)), filter, new SearchControls());

  or

  LdapName name = new LdapName("o=org");
  name.addAll(new LdapName(taintedDN).getRdns());
  ctx.search(new LdapName("ou=people").addAll(name), filter, new SearchControls());

  or

  ctx.search(new LdapName(taintedDN).toString(), filter, new SearchControls());

  or

  ctx.search((Name) new LdapName(taintedDN).clone(), filter, new SearchControls());

• DirContext.search with tainted Rdn, for example:

  ctx.search(new LdapName(List.of(new Rdn(taintedDN))), filter, new SearchControls());

UnboundID

• LDAPConnection.search with tainted DN or filter, for example:

  conn.search(null, taintedDN, null, null, 1, 1, false, taintedFilter);

• Filter.create with tainted filter, for example:

  conn.search(null, dn, null, null, 1, 1, false, Filter.create(taintedFilter));

  or

  conn.search(null, dn, null, null, 1, 1, false, Filter.createNOTFilter(Filter.create(taintedFilter)));

  or

  conn.search(null, dn, null, null, 1, 1, false, Filter.create(taintedFilter).toString());

  or

  StringBuilder sb = new StringBuilder();
  Filter.create(taintedFilter).toNormalizedString(sb);
  conn.search(null, dn, null, null, 1, 1, false, sb.toString());

• SearchRequest with tainted DN or filter, for example:

  SearchRequest s = new SearchRequest(null, taintedDN, null, null, 1, 1, false, taintedFilter);
  conn.search(s);

  or

  SearchRequest s = new SearchRequest(null, taintedDN, null, null, 1, 1, false, taintedFilter);
  conn.search(s.duplicate());

  or

  SearchRequest s = new SearchRequest(null, "", null, null, 1, 1, false, "");
  s.setBaseDN(taintedDN);
  s.setFilter(taintedFilter);
  conn.search(s);

• LDAPConnection.searchForEntry with tainted DN or filter, for example:

  conn.searchForEntry(taintedDN, null, null, 1, false, taintedFilter);

• LDAPConnection.asyncSearch with SearchRequest with tainted DN or filter, for example:

  SearchRequest s = new SearchRequest(null, taintedDN, null, null, 1, 1, false, taintedFilter);
  conn.asyncSearch(s);

Spring LDAP

• LdapTemplate.search, LdapTemplate.authenticate, LdapTemplate.searchForObject, LdapTemplate.findOne, LdapTemplate.find or LdapTemplate.searchForContext with tainted DN or filter, for example:

  templ.search(taintedDN, taintedFilter, 1, false, null);

• LdapNameBuilder with tainted DN, for example:

  templ.authenticate(LdapNameBuilder.newInstance(taintedDN).build(), filter, "password");

  or

  templ.authenticate(LdapNameBuilder.newInstance().add(taintedDN).build(), filter, "password");

• LdapQueryBuilder with tainted filter or DN, for example:

  templ.findOne(LdapQueryBuilder.query().filter(taintedFilter), null);

  or

  templ.find(LdapQueryBuilder.query().base(taintedDN).base(), null, null, null);

  or

  templ.searchForContext(LdapQueryBuilder.query().base(taintedDN).where("uid").is("test"));

• LdapUtils.newLdapName with tainted DN, for example:

  templ.find(LdapUtils.newLdapName(taintedDN), filter, null, null);

• HardcodedFilter with tainted filter, for example:

  templ.find(dn, new HardcodedFilter(taintedFilter), null, null);

  or

  templ.searchForContext(LdapQueryBuilder.query().filter(new HardcodedFilter(taintedFilter)));

  or

  templ.search(dn, new HardcodedFilter(taintedFilter).toString(), 1, false, null);

  or

  StringBuffer sb = new StringBuffer();
  new HardcodedFilter(taintedFilter).encode(sb);
  templ.search(dn, sb.toString(), 1, false, null);

Apache LDAP API

• LdapConnection.search with tainted DN or filter, for example:

  conn.search(taintedDN, taintedFilter, null);

• Dn with tainted DN, for example:

  conn.search(new Dn(taintedDN).getName(), filter, null);

• SearchRequest with tainted DN or filter, for example:

  SearchRequest s = new SearchRequestImpl();
  s.setBase(new Dn(taintedDN));
  s.setFilter(taintedFilter);
  conn.search(s);

  or

  SearchRequest s = new SearchRequestImpl();
  s.setBase(new Dn(taintedDN));
  conn.search(s.getBase(), filter, null);

[aschackmull]

QL code generally looks good, but I've left a few inline comments.

[ggolawski]

I refactored the QL code as you suggested.

BTW, I have a Java class with various types of LDAP injection vulnerabilities as well as good code. I use it to test this query. I'm not sure how can I use it here, so I'll just link it: https://gist.github.com/ggolawski/bb8e501253e850c1c228f88b86fc7acc
The parameter names are little bit cryptic - this let me quickly figure out which test has failed. In general, parameters and methods always contain 'Ok' or 'Bad' in their names, depending whether the query should flag them or not.
All the tests pass after the refactoring :)

[felicitymay]

Thanks for including a help topic with your query - it's a great help to users. It's also good to have lots of examples and references.

I've made some suggestions to make the help more consistent with existing topics, and with the aim of simplifying some of the longer sentences. In one place, I wasn't entirely sure of your original meaning, so please review that suggestion with particular care. The rest of the topic seemed very clear.

[aschackmull]

Thanks for the refactor - QL code is looking good now. The test that you've linked in the gist is definitely something that we want to include - we have a unit test framework where it should fit in, but the tests should be self-contained without dependencies to third-party libraries, which means that for cases like these we usually add interface stubs. I'll look into taking care of that, so no need to worry about it (also, since you can't quite execute the unit tests yet - the external tooling for that will be available soon!). As soon as I've got the test ready I'll let you know, so you can review the expected output.

[ggolawski]

Thanks @aschackmull for taking care of unit tests. I'll check the output once the tests are ready.

[aschackmull]

I've added your unit test along with the necessary library stubs, options file, and .expected file as a PR against your PR: ggolawski#3
I've added a couple of comments where it looks like there are false positives and false negatives. Could you check why this might be the case?

[ggolawski]

If I count correctly, there're 2 false positives and 2 false negatives. They all can be fixed by slightly modifying the stubs. I've commented the relevant code in ggolawski#3 with the details.
I've run the test with modified stubs and all 4 issues were corrected. Please try this in your environment.
Or should I merge ggolawski#3 and apply the fixes?

[aschackmull]

I've fixed it on ggolawski#3.

[felicitymay]

From the documentation point of view, this all looks ready to merge 👍

[ggolawski]

Thanks for fixing the tests. I've merged ggolawski#3.
I've also renamed CWE-90 directory to CWE-090.

[elecharny]

Hi,

where in the coder could I find the value of variable such taintedDN or taintedFilter ? I quickly had a look at the PR, wasn't able to find it.

Thanks !

[ggolawski]

Hi,

I guess you're referring to the examples from the PR description.
taintedDn and taintedFilter just represent untrusted values provided by user. There's LDAP injection If such user provided value ends up as DN or filter parameter to a method that executes the LDAP query.

This CodeQL query detects such flows by statically analyzing the code. It doesn't check the actual variable values - this would be dynamic analysis and would require the code to be executed.

I hope it helps.

[elecharny]

Hi,

do you have example of such tainted values ? For instance, if you look on the internet, you find things like (&(uid=xxxx)(?)(userPassword=yyy)) which simply won't be accepted by Apache LDAP API (we do reject such construct which is not valid by RFC 4515, so it can't be used to try some LDAP injection to gain access to a web site, for instance.

I'm asking because I'm interested in seeing if there are means to abuse the Apache LDAP API and if so, to try to fix it (if possible).

Many thanks !

[ggolawski]

Hi,

You can find some payloads here: https://github.com/swisskyrepo/PayloadsAllTheThings/tree/master/LDAP%20Injection

The first example results in 2 valid filters and I'm not sure if it would be accepted by Apache LDAP API:

user  = *)(uid=*))(|(uid=*
pass  = password
query = "(&(uid=*)(uid=*)) (|(uid=*)(userPassword={MD5}X03MO1qnZdYdgyfeuILPmQ==))"

But the second example results in a single valid filter which would be accepted, wouldn't it?:

user  = admin)(!(&(1=0
pass  = q))
query = (&(uid=admin)(!(&(1=0)(userPassword=q))))

[elecharny]

Hi,

First, the userPassword attribute is rarely stored in clear text (for obvious security reasons). The standard form is hashed, and the server compares a hashed version of what is provided by a user during a bind with what is stored. Any request that fetches this attribute will returns the hashed value, which will only be 'good' if you want to conduct a rainbow attack on it.
But a well tuned LDAP server will never return the userPassword value, just because of the first point : avoiding a rainbow attack.

Regarding the samples, a request containing something like (1=0) will be rejected by a server that checks the attributes as '1' is not a valid attributeType.
The first request will be rejected because it is actually split in 2 filters (the first one being (&(uid=)(uid=)) and the second one (|(uid=*)(userPassword={MD5}X03MO1qnZdYdgyfeuILPmQ==)))

Regardless, what is troubling is to expect an application using a login form with user/password fields to send a search request, instead of doing a bind, which is the base operation when it comes to validate a login... Of couse, we are not short of such bad web application out there ;-)

[ggolawski]

Exactly, everything depends on the application and how the results are interpreted :-)

In the second example in my previous comment, if the code would execute the search and then check if it returned anything (assuming that the credentials are correct if it did), it would be possible to bypass this authentication. And (1=0) could be substituted with (uid=x) in the payload :-)

In general, using user provided input to construct search filter without sanitizing it is always a bad idea.