CheatSheetSeries icon indicating copy to clipboard operation
CheatSheetSeries copied to clipboard

Update: LDAP_Injection_Prevention_Cheat_Sheet

Open einhirn opened this issue 3 months ago • 2 comments

What is missing or needs to be updated?

After a casual reading, the Cheat Sheet seems to suggests that even passwords should always be escaped. When this is taken at face value, it can cause all kinds of issues, mainly stuff not working as expected any more (see https://github.com/dexidp/dex/issues/3433).

The Safe Java Escaping Example wrongly assumes that a password check against LDAP is done via LDAP filter and escapes the password, when actually password checking against an ldap server is (or at least should be) exclusively done via BIND request which safely accepts the password in unfiltered plaintext.

How should this be resolved?

The usual methods for password authentication against LDAP should be mentioned:

  • Single-Step Auth:
    • BIND-Request with DN constructed from template like uid=${USERID},ou=people,dc=example,dc=com. Whatever is inserted in that template should be escaped as needed. Password from user input is given to BIND call as is.
  • Three-Step-Auth:
    • BIND with credentials limited to searching - these would be configured in the app.
    • SEARCH LDAP for users DN like (&(uid=${USERID})(deactivated=0)) or something. Again, anything inserted into that query should be escaped as needed
    • BIND with DN from search result above, if any. Password from user input is given to BIND call as is.

These two both assume the LDAP server to require (non-anonymous) BIND before allowing access to user data, which I think is (or should be) default or at least best practice. So Enabling Bind Authentication should be moved up in the document's focus, i.e. "To prevent LDAP Injection, first the LDAP server needs to be configured properly...". Even if it's out of scope of this document, proper safety precautions should be mentioned - "Simple BIND transmits plain text passwords to the server, so it should only be used with an encrypted connection" or "If you really need to use unencrypted connection, make sure to only use SASL BIND with challenge protocols etc. to prevent password sniffing."

The Safe Java Escaping Example should be updated to reflect either one of the above methods and no longer use the password in a search filter.

einhirn avatar Apr 08 '24 13:04 einhirn

This cheatsheet can use a lot of changes. I like your ideas so far. Would you care to submit PR's?

jmanico avatar Apr 08 '24 20:04 jmanico

I'll look into it and see when I might find some time to do that... I tend to overthink what I write on a word for word level, so it takes a lot of time to write something, especially something public in the field of security 😬

einhirn avatar Apr 11 '24 16:04 einhirn