Fix LDAP query built from user-controlled sources SingularityLDAPDatastore

[odaysec]

https://github.com/HubSpot/Singularity/blob/15688f486fc9286878eff40b51789c88bd6899d5/SingularityService/src/main/java/com/hubspot/singularity/auth/datastore/SingularityLDAPDatastore.java#L161-L161

To fix the issue need to ensure that the user input is properly sanitized before being used in the LDAP query. The best approach is to use a library that provides safe methods for constructing LDAP queries, such as the Apache Directory API. Specifically, we can use the FilterBuilder class to safely encode the user input when constructing the LDAP filter. This eliminates the risk of LDAP injection.

The changes will involve:

1. Replacing the use of String.format(configuration.getUserFilter(), user) with a safe filter constructed using FilterBuilder.equal.
2. Ensuring that the user input is properly encoded before being used in the LDAP query.

---

If an LDAP query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious LDAP queries.

POC

In the following the code accepts an "organization name" and a "username" from the user, which it uses to query LDAP. The first concatenates the unvalidated and unencoded user input directly into both the DN (Distinguished Name) and the search filter used for the LDAP query. A malicious user could provide special characters to change the meaning of these queries, and search for a completely different set of values. The LDAP query is executed using Java JNDI API.

The second uses the OWASP ESAPI library to encode the user values before they are included in the DN and search filters. This ensures the meaning of the query cannot be changed by a malicious user.

import javax.naming.directory.DirContext;
import org.owasp.esapi.Encoder;
import org.owasp.esapi.reference.DefaultEncoder;

public void ldapQueryBad(HttpServletRequest request, DirContext ctx) throws NamingException {
  String organizationName = request.getParameter("organization_name");
  String username = request.getParameter("username");

  // BAD: User input used in DN (Distinguished Name) without encoding
  String dn = "OU=People,O=" + organizationName;

  // BAD: User input used in search filter without encoding
  String filter = "username=" + userName;

  ctx.search(dn, filter, new SearchControls());
}

public void ldapQueryGood(HttpServletRequest request, DirContext ctx) throws NamingException {
  String organizationName = request.getParameter("organization_name");
  String username = request.getParameter("username");

  // ESAPI encoder
  Encoder encoder = DefaultEncoder.getInstance();

  // GOOD: Organization name is encoded before being used in DN
  String safeOrganizationName = encoder.encodeForDN(organizationName);
  String safeDn = "OU=People,O=" + safeOrganizationName;

  // GOOD: User input is encoded before being used in search filter
  String safeUsername = encoder.encodeForLDAP(username);
  String safeFilter = "username=" + safeUsername;
  
  ctx.search(safeDn, safeFilter, new SearchControls());
}

The third uses Spring LdapQueryBuilder to build an LDAP query. In addition to simplifying the building of complex search parameters, it also provides proper escaping of any unsafe characters in search filters. The DN is built using LdapNameBuilder, which also provides proper escaping.

import static org.springframework.ldap.query.LdapQueryBuilder.query;
import org.springframework.ldap.support.LdapNameBuilder;

public void ldapQueryGood(@RequestParam String organizationName, @RequestParam String username) {
  // GOOD: Organization name is encoded before being used in DN
  String safeDn = LdapNameBuilder.newInstance()
    .add("O", organizationName)
    .add("OU=People")
    .build().toString();

  // GOOD: User input is encoded before being used in search filter
  LdapQuery query = query()
    .base(safeDn)
    .where("username").is(username);

  ldapTemplate.search(query, new AttributeCheckAttributesMapper());
}

The fourth uses UnboundID classes, Filter and DN, to construct a safe filter and base DN.

import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.RDN;
import com.unboundid.ldap.sdk.Filter;

public void ldapQueryGood(HttpServletRequest request, LDAPConnection c) {
  String organizationName = request.getParameter("organization_name");
  String username = request.getParameter("username");

  // GOOD: Organization name is encoded before being used in DN
  DN safeDn = new DN(new RDN("OU", "People"), new RDN("O", organizationName));

  // GOOD: User input is encoded before being used in search filter
  Filter safeFilter = Filter.createEqualityFilter("username", username);
  
  c.search(safeDn.toString(), SearchScope.ONE, safeFilter);
}

The fifth shows how to build a safe filter and DN using the Apache LDAP API.

import org.apache.directory.ldap.client.api.LdapConnection;
import org.apache.directory.api.ldap.model.name.Dn;
import org.apache.directory.api.ldap.model.name.Rdn;
import org.apache.directory.api.ldap.model.message.SearchRequest;
import org.apache.directory.api.ldap.model.message.SearchRequestImpl;
import static org.apache.directory.ldap.client.api.search.FilterBuilder.equal;

public void ldapQueryGood(HttpServletRequest request, LdapConnection c) {
  String organizationName = request.getParameter("organization_name");
  String username = request.getParameter("username");

  // GOOD: Organization name is encoded before being used in DN
  Dn safeDn = new Dn(new Rdn("OU", "People"), new Rdn("O", organizationName));

  // GOOD: User input is encoded before being used in search filter
  String safeFilter = equal("username", username);
  
  SearchRequest searchRequest = new SearchRequestImpl();
  searchRequest.setBase(safeDn);
  searchRequest.setFilter(safeFilter);
  c.search(searchRequest);
}

References

LDAP Injection Prevention Cheat Sheet OWASP ESAPI LdapQueryBuilder LdapNameBuilder Understanding and Defending Against LDAP Injection Attacks CWE-90