ldapsdk icon indicating copy to clipboard operation
ldapsdk copied to clipboard

Search filter with escaped comma

Open briandignan opened this issue 10 years ago • 3 comments

Some of our Active Directory accounts have commas in the CN field. For example: Smith, James K.

When I query the distinguishedName (which contains the CN) for this account, it returns this: cn=Smith\, James K.,ou=West,dc=MyDomain,dc=com.

However, if I try to use this string to construct a new filter for a subsequent query the groups that the account is a member of: (&(objectClass=group)(member=cn=Smith\, James K.,ou=West,dc=MyDomain,dc=com))

Filter.readEscapedHexString throws an exception:

Exception in thread "main" LDAPException(resultCode=87 (filter error), errorMessage='Invalid hex character ',' encountered at position 38.')
    at com.unboundid.ldap.sdk.Filter.readEscapedHexString(Filter.java:1854)
    at com.unboundid.ldap.sdk.Filter.create(Filter.java:1630)
    at com.unboundid.ldap.sdk.Filter.parseFilterComps(Filter.java:1757)
    at com.unboundid.ldap.sdk.Filter.create(Filter.java:1159)
    at com.unboundid.ldap.sdk.Filter.create(Filter.java:1070)
    at com.unboundid.ldap.sdk.SearchRequest.<init>(SearchRequest.java:301)

It works when I modify the first switch statement in Filer.readEscapedHexString to contain this:

  case ',':
    b = (byte) 0x5c; // backslash
    buffer.append( b );
    b = (byte) 0x2c; // comma
    buffer.append( b );
    return startPos + 1;

But I don't know the code well to determine if this would create other bugs. Do you know if there are any other ways around this problem?

briandignan avatar Dec 18 '15 12:12 briandignan

In general, you should not create search filters by constructing their string representations. Instead, you should programmatically construct them from their individual components. In the case of the filter you have listed, instead of trying to construct its string representation, you should build it like:

Filter filter = Filter.createANDFilter(
     Filter.createEqualityFilter("objectClass", "group"),
     Filter.createEqualityFilter("member", userEntry.getDN()));

Using this approach instead of concatenating strings provides several benefits, including:

  • The LDAP SDK will automatically escape any special characters that might require it (commas, asterisks, parentheses, etc.).
  • Related to the above, constructing filters programmatically rather than using string concatenation protects you against LDAP injection attacks. LDAP is more resistant than SQL in this regard, but there are still some use cases in which a malicious user might be able to create custom input that tricks the application into requesting more information than you intend to allow. Using the programmatic methods for constructing filters ensures that this can't happen because all of the special characters required by these kinds of exploits will be properly excaped.
  • LDAP filters aren't sent as strings over the wire, but use a binary encoding that more closely matches what you get from the programmatic construction methods. If you provide a filter as a string representation, the LDAP SDK has to do more work to parse that string into the binary representation.

dirmgr avatar Dec 18 '15 19:12 dirmgr

And for the record, the correct way to escape a comma in a search filter is with "\5c,". That is, you escape the backslash as "\5c" and leave the comma alone. So the correct string representation of the filter you're trying to use is:

(&(objectClass=group)(member=cn=Smith\5c, James K.,ou=West,dc=MyDomain,dc=com))

The syntax for escaping filters is different from the syntax for escaping DNs. I didn't write the specs, but that's the way it is.

And although it's not applicable in this case, if you're not sure how to properly escape a DN, you can also programmatically construct a DN and have the LDAP SDK do all of the escaping for you. The code to create the DN listed above would be:

DN dn = new DN(
    new RDN("cn", "Smith, James K."),
    new RDN("ou", "West"),
    new RDN("dc", "MyDomain"),
    new RDN("dc", "com"));

dirmgr avatar Dec 18 '15 22:12 dirmgr

Thank you Neil! That worked.

briandignan avatar Dec 19 '15 03:12 briandignan