diff --git a/dal/tests/test_flows.py b/dal/tests/test_flows.py index 88d4aef..a440320 100644 --- a/dal/tests/test_flows.py +++ b/dal/tests/test_flows.py @@ -73,6 +73,7 @@ class RegisterTests(LdapTestCase): def test_validation_errors(self): self.assert_error(self.form(username=""), "Please supply a username") self.assert_error(self.form(username="Bad Name"), "lowercase letters") + self.assert_error(self.form(username="pipe|name"), "lowercase letters") self.assert_error(self.form(password2="different"), "match") self.assert_error(self.form(email="not-an-email"), "email address is invalid") self.assert_error(self.form(lastname=""), "firstname and lastname") @@ -251,9 +252,8 @@ class ResetPasswordTests(LdapTestCase): self.assertFalse(self.can_bind(victim, "attacker-1234", ou="users"), "account takeover without any token") - @expectedFailure - def test_bug_ldap_filter_injection_in_reset(self): - """CRITICAL: user='*' matches the first entry in the whole tree.""" + def test_ldap_filter_injection_in_reset(self): + """user='*' must not match the first entry in the whole tree.""" self.make_ldap_user(password="victim-pass-1234") self.client.post("/reset/", { "user": "*", "password1": "attacker-1234", "password2": "attacker-1234"}) diff --git a/dal/ungleich_ldap.py b/dal/ungleich_ldap.py index 1dedbc9..c594399 100644 --- a/dal/ungleich_ldap.py +++ b/dal/ungleich_ldap.py @@ -4,6 +4,8 @@ import random import ldap3 from django.conf import settings +from ldap3.utils.conv import escape_filter_chars +from ldap3.utils.dn import escape_rdn import logging logger = logging.getLogger(__name__) @@ -93,7 +95,7 @@ class LdapManager: try: uid = user # user.encode("utf-8") conn.add("uid={uid},{customer_dn}".format( - uid=uid, customer_dn=settings.LDAP_CUSTOMER_DN + uid=escape_rdn(uid), customer_dn=settings.LDAP_CUSTOMER_DN ), ["inetOrgPerson", "posixAccount", "ldapPublickey"], { @@ -216,7 +218,7 @@ class LdapManager: result = conn.search( search_base=search_base, search_filter=search_filter if len(search_filter)> 0 else - '(uid={uid})'.format(uid=uid), + '(uid={uid})'.format(uid=escape_filter_chars(uid)), attributes=attributes ) entries = conn.entries @@ -234,7 +236,7 @@ class LdapManager: conn = self.get_admin_conn() try: return_val = conn.delete( - ("uid={uid}," + settings.LDAP_CUSTOMER_DN).format(uid=uid), + ("uid={uid}," + settings.LDAP_CUSTOMER_DN).format(uid=escape_rdn(uid)), ) msg = "success" except Exception as ex: diff --git a/dal/views.py b/dal/views.py index 2a81cfb..3d814f8 100644 --- a/dal/views.py +++ b/dal/views.py @@ -37,7 +37,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin # Small helper used for registration. def is_username_valid(username): - return re.fullmatch(r"^[a-z|0-9|\-|_]+$", username) + return re.fullmatch(r"[a-z0-9_-]+", username) admin_seed = config('ADMIN_SEED') admin_name = config('ADMIN_NAME')