From 03d09f0ba20774db9369a2006e705060bd690cea Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Sun, 26 Nov 2017 22:16:56 +0100 Subject: [PATCH] Fix resource leaks in error cases 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 --- Modules/LDAPObject.c | 75 +++++++++++++++++++++++++++++++++----------- Tests/t_cext.py | 47 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/Modules/LDAPObject.c b/Modules/LDAPObject.c index f1ad0d4..b066cdf 100644 --- a/Modules/LDAPObject.c +++ b/Modules/LDAPObject.c @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 */ @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); @@ -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 ); diff --git a/Tests/t_cext.py b/Tests/t_cext.py index d4740d0..5aeb03d 100644 --- a/Tests/t_cext.py +++ b/Tests/t_cext.py @@ -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 @@ -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()