noirello / bonsai

Simple Python 3 module for LDAP, using libldap2 and winldap C libraries.
MIT License
116 stars 32 forks source link

LDAP injection #60

Closed Dawoodkhorsandi closed 2 years ago

Dawoodkhorsandi commented 2 years ago

Hi

I was trying to use this two functions escape_filter_chars and escape_dn_chars from python-ldap to prevent LDAP injections. But I've got bonsai.errors.LDAPError: Bad search filter. (0xFFF9 [-7]).

I've read the docs already but I did not find any utility functions to prevent LDAP injection in bonsai.

What are the workarounds?

Thanks in advance Dawood

Dawoodkhorsandi commented 2 years ago

Oh I found this now https://github.com/noirello/bonsai/blob/master/src/bonsai/utils.py

Dawoodkhorsandi commented 2 years ago

But the Bad Search Filter Error remained.

So now the question is where bonsai.utils.escape_attribute_value and bonsai.utils.bonsai.utils.escape_attribute_value should be used? Because they're not used by bonsai internallly.

Are they meant to be used to prevent LDAP injection?

noirello commented 2 years ago

Hi, could you give me a full search example?

Dawoodkhorsandi commented 2 years ago
import asyncio

from bonsai import LDAPClient
from bonsai.utils import escape_filter_exp

async def run_me():
    ldap_host_url = 'ldap://hostname'
    admin_dn = 'LDAP admin dn'
    admin_password = 'LDAP admin password'
    base_dn = 'valid base dn'
    scope = 1
    offset = 0
    ldap_search_fields = ['dn', 'sn', 'createTimestamp']

    ldap_filter = '(&(mail=john_doe@example.com)(cn=john))'
    print(f'Filter before been escaped -> {ldap_filter}')
    ldap_filter = escape_filter_exp(ldap_filter)
    print(f'Filter after been escaped -> {ldap_filter}')

    client = LDAPClient(ldap_host_url)
    client.set_credentials('SIMPLE', user=admin_dn, password=admin_password)
    connection = await client.connect(is_async=True)
    search_result = await connection.virtual_list_search(base_dn,
                                                         scope,
                                                         filter_exp=ldap_filter,
                                                         attrlist=ldap_search_fields,
                                                         sort_order=['-createTimestamp'],
                                                         offset=offset,
                                                         before_count=0,
                                                         after_count=10,
                                                         )
    return search_result

asyncio.run(run_me())

Traceback

Filter before been escaped -> (&(mail=john_doe@example.com)(cn=john))
Filter after been escaped -> \28&\28mail=john_doe@example.com\29\28cn=john\29\29

Traceback (most recent call last):
  File "/home/dawood/repositories/abrnoc/backoffice/ldap_injection_prevention.py", line 36, in <module>
    asyncio.run(run_me())
  File "/usr/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/home/dawood/repositories/abrnoc/backoffice/ldap_injection_prevention.py", line 24, in run_me
    search_result = await connection.virtual_list_search(base_dn,
  File "/home/dawood/repositories/abrnoc/backoffice/venv/lib/python3.8/site-packages/bonsai/ldapconnection.py", line 203, in virtual_list_search
    return self.__base_search(
  File "/home/dawood/repositories/abrnoc/backoffice/venv/lib/python3.8/site-packages/bonsai/ldapconnection.py", line 95, in __base_search
    msg_id = super().search(
bonsai.errors.LDAPError: Bad search filter. (0xFFF9 [-7])

I guess the escape_filter_exp should be used on individual values like john_doe@example.com not the whole filter string.

noirello commented 2 years ago

Yes, you are right: escape_filter_exp should only used on untrusted input parameters.

Dawoodkhorsandi commented 2 years ago

Thanks.

I think the example provided in the API documentation will mislead the readers.

>>> import bonsai
>>> bonsai.escape_filter_exp("(objectclass=*)")
'\\28objectclass=\\2A\\29'

From the docs, it seems that it should be used on the whole filter. Even the function name and its parameter name implies it escapes the whole filter expression.

noirello commented 2 years ago

You're right again. I'll update the docs with better examples.