[Esapi-dev] LDAP Injection

[Kevin W. Wall]

On 12/28/2010 11:16 PM, Jeff Williams (aka, the Amazing Kreskin) wrote:

> Your next revelation will be that there are several contexts *within* the
> LDAP syntax, and you're going to want several different methods.

Wow! You're predictive powers *are* amazing! :)

Actually, I can think of at least three contexts that are a must,
and maybe on a fourth:
	1) The DN (we already have that covered via Encoder.encodeForDN())
	2) An LDAP search filter
	3) Places where LDIF format is used (e.g., if you were to call
	   ldapmodify from the command line or do similar interactions via API
	   calls)
	4) X.500 addressing format as used with DNs on X.509 client certs.

Based on the implementation in DefaultEncoder, it appears as though the *intent*
was to cover *both* of these later two cases via the Encoder.encodeForLDAP()
interface method. I mention "intent" because based on the DefaultEncoder code,
and Javadoc comments for encodeForLDAP(), it seems to *only* cover the case
for #2 but not #3.

In fact, I was mentioning this to Jim just yesterday and I told him that
I thought that we should deprecate the method name encodeForLDAP() and
rename the method to something like encodeForLDAPSearchFilter() to
clear up any ambiguity in its intent. Then perhaps we could add a 3rd
method called encodeForLDIF().

Jim also requested that I pull some of the new information that I responded
in an an off-list to him and Jim temporarily posted at
<http://www.owasp.org/index.php/LDAP_injection_defense> (my bad; I meant
to most this to the ESAPI Dev list too), and add some of that new info to
<http://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java>, which
is much more comprehensive. (I will do that sometime soon; hopefully before
year end, and than ask someone to review it.)

However, I think that the contexts for #2 and #3 above clearly need to
be dealt with separately. If one is writing an LDAP search filter for
an LDAP telephonenumber attribute, then clearly one needs to escape
any '(' and ')' characters appearing in a phone number originating from
user input to prevent LDAP injection. However, if one is inserting /
changing a user's telephone number into LDAP, you don't want to escape
these characters otherwise the phone # would end up in LDAP incorrectly.
The former uses the LDAP search filter context mentioned in #2 whereas
the latter would use the LDIF context mentioned in #3.

There is also a lesser used context (#4) with LDAP concerned with storing
X.509 client-certificate information which I described in the email that Jim
pulled into the new Wiki page (LDAP_injection_defense).In that scenario,
at least used with Apache's mod_ssl, the DNs for client certificates use the
X.500 addressing scheme, where '/' rather than ',' is used as the delimiters
of the DN. E.g., from mod_ssl, I would see something like this for the
client-certificate DN
	CN=wallk/OU=Qwest IT/DC=qwest/DC=com
rather than
	CN=wallk, OU=Qwest IT, DC=qwest, DC=com

Since there is often an interaction with LDAP and client certificates which
can (at least with Apache mod_ssl) use X.500 addressing, we may wish to take
this into account as well when dealing with X.509 DNs. (From an interface
point of view, this could mean that we just add an
Encoder.encodeForCertificateDN() method, but if we decide to create a
separate codec to handle LDAP, the question would be should this method
be covered there or go to a separate codec for X.500? [Note that a separate
LDAP codec is already indicated as desirable in a couple of TODO comments
in DefaultEncoder.)

> Then you're going to suggest that it's too complicated and you want an
> automatic escaping system based on some crazy template system.

I don't think it's going to be *that* complicated. It's much simpler than
with HTML.

-kevin
-- 
Kevin W. Wall
"The most likely way for the world to be destroyed, most experts agree,
is by accident. That's where we come in; we're computer professionals.
We cause accidents."        -- Nathaniel Borenstein, co-creator of MIME