Skip to content

Commit

Permalink
Fix resource leaks in error cases
Browse files Browse the repository at this point in the history 
On several occasions LDAPObject methods did not clean up LDAPControls
and other resources when LDAPControls_from_object() fails. This bug
would lead to memory leaks in case a server control or client control
could not be handled correctly.

Closes: #18
Signed-off-by: Christian Heimes <cheimes@redhat.com>
  • Loading branch information
Christian Heimes committed Nov 27, 2017
1 parent a06267f commit 03d09f0
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 18 deletions.
75 changes: 57 additions & 18 deletions Modules/LDAPObject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,10 @@ l_ldap_unbind_ext( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -392,8 +394,10 @@ l_ldap_abandon_ext( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -434,13 +438,18 @@ l_ldap_add_ext( LDAPObject* self, PyObject *args )
return NULL;

if (!PyNone_Check(serverctrls)) {
if (!LDAPControls_from_object(serverctrls, &server_ldcs))
if (!LDAPControls_from_object(serverctrls, &server_ldcs)) {
LDAPMods_DEL( mods );
return NULL;
}
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPMods_DEL( mods );
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -482,8 +491,10 @@ l_ldap_simple_bind( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -631,8 +642,10 @@ l_ldap_sasl_bind_s( LDAPObject* self, PyObject* args )
return NULL;
}
if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -695,8 +708,10 @@ l_ldap_sasl_interactive_bind_s( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

/* now we extract the sasl mechanism from the SASL Object */
Expand Down Expand Up @@ -755,8 +770,10 @@ l_ldap_cancel( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -801,8 +818,10 @@ l_ldap_compare_ext( LDAPObject* self, PyObject *args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -842,8 +861,10 @@ l_ldap_delete_ext( LDAPObject* self, PyObject *args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -884,13 +905,18 @@ l_ldap_modify_ext( LDAPObject* self, PyObject *args )
return NULL;

if (!PyNone_Check(serverctrls)) {
if (!LDAPControls_from_object(serverctrls, &server_ldcs))
if (!LDAPControls_from_object(serverctrls, &server_ldcs)) {
LDAPMods_DEL( mods );
return NULL;
}
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPMods_DEL( mods );
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -934,8 +960,10 @@ l_ldap_rename( LDAPObject* self, PyObject *args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -1132,13 +1160,18 @@ l_ldap_search_ext( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(serverctrls)) {
if (!LDAPControls_from_object(serverctrls, &server_ldcs))
if (!LDAPControls_from_object(serverctrls, &server_ldcs)) {
free_attrs( &attrs, attrs_seq);
return NULL;
}
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
free_attrs( &attrs, attrs_seq);
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -1182,8 +1215,10 @@ l_ldap_whoami_s( LDAPObject* self, PyObject* args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -1290,8 +1325,10 @@ l_ldap_passwd( LDAPObject* self, PyObject *args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down Expand Up @@ -1340,8 +1377,10 @@ l_ldap_extended_operation( LDAPObject* self, PyObject *args )
}

if (!PyNone_Check(clientctrls)) {
if (!LDAPControls_from_object(clientctrls, &client_ldcs))
if (!LDAPControls_from_object(clientctrls, &client_ldcs)) {
LDAPControl_List_DEL( server_ldcs );
return NULL;
}
}

LDAP_BEGIN_ALLOW_THREADS( self );
Expand Down
47 changes: 47 additions & 0 deletions Tests/t_cext.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ def test_search_ext_all(self):
self.assertEqual(msgid, m)
self.assertEqual(ctrls, [])

def test_invalid_search_filter(self):
l = self._open_conn()
with self.assertRaises(_ldap.FILTER_ERROR):
l.search_ext(
self.server.suffix, _ldap.SCOPE_SUBTREE, 'bogus filter expr'
)

def test_add(self):
"""
test add operation
Expand Down Expand Up @@ -726,6 +733,46 @@ def test_invalid_credentials(self):
else:
self.fail("expected INVALID_CREDENTIALS, got %r" % r)

# TODO: test_extop

def assertInvalidControls(self, func, *args, **kwargs):
post = kwargs.pop('post', ())
self.assertFalse(kwargs)
# last two args are serverctrls, clientctrls
with self.assertRaises(TypeError) as e:
func(*(args + (object, None) + post))
self.assertEqual(e.exception.args, ('expected a list', object))
with self.assertRaises(TypeError) as e:
func(*(args + (None, object) + post))
self.assertEqual(e.exception.args, ('expected a list', object))

def test_invalid_controls(self):
l = self._open_conn()
self.assertInvalidControls(l.simple_bind, "", "")
self.assertInvalidControls(l.whoami_s)
self.assertInvalidControls(l.passwd, 'dn', 'initial', 'changed')
self.assertInvalidControls(l.add_ext, 'dn', [('cn', b'cn')])
self.assertInvalidControls(
l.modify_ext, 'dn', [(_ldap.MOD_ADD, 'attr', [b'value'])])
self.assertInvalidControls(l.compare_ext, 'dn', 'val1', 'val2')
self.assertInvalidControls(
l.rename, 'dn', 'newdn', 'container', False)
self.assertInvalidControls(
l.search_ext, 'dn', _ldap.SCOPE_SUBTREE, '(objectClass=*)',
None, 1)
self.assertInvalidControls(l.delete_ext, 'dn')
m = l.search_ext(
self.server.suffix, _ldap.SCOPE_SUBTREE, '(objectClass=*)')
self.assertInvalidControls(l.abandon_ext, m)
self.assertInvalidControls(l.cancel, 0)
self.assertInvalidControls(l.extop, 'oid', 'value')
if hasattr(l, 'sasl_bind_s'):
self.assertInvalidControls(l.sasl_bind_s, 'dn', 'MECH', 'CRED')
if hasattr(l, 'sasl_interactive_bind_s'):
self.assertInvalidControls(
l.sasl_interactive_bind_s, 'who', 'SASLObject', post=(1,))
self.assertInvalidControls(l.unbind_ext)


if __name__ == '__main__':
unittest.main()

0 comments on commit 03d09f0

Please sign in to comment.