Hi, the attached patches fix <https://fedorahosted.org/freeipa/ticket/4651>.
Honza -- Jan Cholasta
>From 7c9436b86bc886c4644ef2e4b4ee59d3832434ac Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:44:05 +0000 Subject: [PATCH 1/6] Fix possible NULL dereference in ipa-kdb https://fedorahosted.org/freeipa/ticket/4651 --- daemons/ipa-kdb/ipa_kdb_mspac.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 084b689..c8f6c76 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -1888,9 +1888,11 @@ void get_authz_data_types(krb5_context context, krb5_db_entry *entry, } ipactx = ipadb_get_context(context); - gcfg = ipadb_get_global_config(ipactx); - if (gcfg != NULL) - tmp = gcfg->authz_data; + if (ipactx != NULL) { + gcfg = ipadb_get_global_config(ipactx); + if (gcfg != NULL) + tmp = gcfg->authz_data; + } if (ipactx == NULL || tmp == NULL) { krb5_klog_syslog(LOG_ERR, "No default authorization data types " \ "available, no authorization data will " \ -- 2.1.0
>From 28248e92d4054ed69b2bb6044466af9094040b76 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:46:19 +0000 Subject: [PATCH 2/6] Fix memory leaks in ipa-extdom-extop https://fedorahosted.org/freeipa/ticket/4651 --- .../ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c index df04347..20fdd62 100644 --- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c +++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c @@ -340,7 +340,8 @@ static int pack_ber_user(enum response_types response_type, ber = ber_alloc_t( LBER_USE_DER ); if (ber == NULL) { - return LDAP_OPERATIONS_ERROR; + ret = LDAP_OPERATIONS_ERROR; + goto done; } ret = ber_printf(ber,"{e{ssii", response_type, domain_name, short_user_name, @@ -449,14 +450,15 @@ static int pack_ber_group(enum response_types response_type, ber = ber_alloc_t( LBER_USE_DER ); if (ber == NULL) { - return LDAP_OPERATIONS_ERROR; + ret = LDAP_OPERATIONS_ERROR; + goto done; } ret = ber_printf(ber,"{e{ssi", response_type, domain_name, short_group_name, gid); if (ret == -1) { - ber_free(ber, 1); - return LDAP_OPERATIONS_ERROR; + ret = LDAP_OPERATIONS_ERROR; + goto done; } if (response_type == RESP_GROUP_MEMBERS) { @@ -716,7 +718,7 @@ static int handle_sid_request(enum request_types request_type, const char *sid, ret = get_buffer(&buf_len, &buf); if (ret != LDAP_SUCCESS) { - return ret; + goto done; } switch(id_type) { -- 2.1.0
>From 0fbee99901723265c0b213b966e96a2c99c17ef3 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:50:26 +0000 Subject: [PATCH 3/6] Fix various bugs in ipa-opt-counter and ipa-otp-lasttoken Fixes a wrong sizeof argument and unchecked return values. https://fedorahosted.org/freeipa/ticket/4651 --- daemons/ipa-slapi-plugins/ipa-otp-counter/berval.c | 2 +- .../ipa-slapi-plugins/ipa-otp-counter/ipa_otp_counter.c | 14 +++++++++++--- .../ipa-otp-lasttoken/ipa_otp_lasttoken.c | 6 +++++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-otp-counter/berval.c b/daemons/ipa-slapi-plugins/ipa-otp-counter/berval.c index 884e1a2..a2fe592 100644 --- a/daemons/ipa-slapi-plugins/ipa-otp-counter/berval.c +++ b/daemons/ipa-slapi-plugins/ipa-otp-counter/berval.c @@ -48,7 +48,7 @@ berval_new_longlong(long long value) { struct berval *bv; - bv = (struct berval*) slapi_ch_malloc(sizeof(struct berval*)); + bv = (struct berval*) slapi_ch_malloc(sizeof(struct berval)); bv->bv_val = slapi_ch_smprintf("%lld", value); bv->bv_len = strlen(bv->bv_val); diff --git a/daemons/ipa-slapi-plugins/ipa-otp-counter/ipa_otp_counter.c b/daemons/ipa-slapi-plugins/ipa-otp-counter/ipa_otp_counter.c index 24ef9e2..da047d7 100644 --- a/daemons/ipa-slapi-plugins/ipa-otp-counter/ipa_otp_counter.c +++ b/daemons/ipa-slapi-plugins/ipa-otp-counter/ipa_otp_counter.c @@ -50,6 +50,7 @@ #include "berval.h" #include "ldapmod.h" +#include "util.h" #include <limits.h> @@ -140,6 +141,7 @@ normalize_input(LDAPMod ***mods, const char *attr, long long ctr) case LDAP_MOD_REPLACE: case LDAP_MOD_INCREMENT: e++; + /* fall through */ default: c++; } @@ -284,8 +286,12 @@ preop_mod(Slapi_PBlock *pb) cpre = get_counter(epre, attr); if (repl == 0) { - if (normalize_input(&mods, attr, cpre) != 0) - slapi_pblock_set(pb, SLAPI_MODIFY_MODS, mods); + if (normalize_input(&mods, attr, cpre) != 0) { + if (slapi_pblock_set(pb, SLAPI_MODIFY_MODS, mods)) { + LOG_FATAL("slapi_pblock_set failed!\n"); + goto error; + } + } } if (!simulate(mods, attr, cpre, &cpost) && repl == 0) { @@ -316,7 +322,9 @@ preop_mod(Slapi_PBlock *pb) error: rc = LDAP_UNWILLING_TO_PERFORM; slapi_send_ldap_result(pb, rc, NULL, msg, 0, NULL); - slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc); + if (slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc)) { + LOG_FATAL("slapi_pblock_set failed!\n"); + } slapi_ch_free_string(&msg); return rc; diff --git a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c index 94d24ae..d20fca1 100644 --- a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c +++ b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c @@ -44,6 +44,8 @@ #include <libotp.h> #include <time.h> +#include "util.h" + #define PLUGIN_NAME "ipa-otp-lasttoken" #define LOG(sev, ...) \ slapi_log_error(SLAPI_LOG_ ## sev, PLUGIN_NAME, \ @@ -100,7 +102,9 @@ static inline int send_error(Slapi_PBlock *pb, int rc, char *errstr) { slapi_send_ldap_result(pb, rc, NULL, errstr, 0, NULL); - slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc); + if (slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc)) { + LOG_FATAL("slapi_pblock_set failed!\n"); + } return rc; } -- 2.1.0
>From 73ff5006b17197b3da574b867160d5683f11a850 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:53:41 +0000 Subject: [PATCH 4/6] Fix memory leak in ipa-pwd-extop Also remove dead code and explicitly mark an ignored return value to prevent false positives in static code analysis. https://fedorahosted.org/freeipa/ticket/4651 --- daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c | 3 +-- daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c index ca021ca..f0346a3 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c @@ -1393,6 +1393,7 @@ done: if (rc != LDAP_SUCCESS) { free(password); free(svcname); + free(enctypes); *_err_msg = err_msg; } else { *_password = password; @@ -1639,7 +1640,6 @@ static int ipapwd_getkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg) krb5_context krbctx = NULL; krb5_error_code krberr; struct berval *extop_value = NULL; - BerElement *ber = NULL; char *service_name = NULL; char *svcname; Slapi_Entry *target_entry = NULL; @@ -1827,7 +1827,6 @@ free_and_return: } free(svals); } - if (ber) ber_free(ber, 1); if (bvp) ber_bvfree(bvp); return SLAPI_PLUGIN_EXTENDED_SENT_RESULT; diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c index 2bfcf10..cbb4536 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c @@ -86,7 +86,7 @@ bool sync_request_handle(Slapi_ComponentId *plugin_id, Slapi_PBlock *pb, } /* Decode the optional token DN. */ - ber_scanf(ber, "a", &token_dn); + (void)ber_scanf(ber, "a", &token_dn); /* Process the synchronization. */ success = false; -- 2.1.0
>From c2b5e4887be6c7e7bd82466852d79adf848fda63 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:59:08 +0000 Subject: [PATCH 5/6] Fix memory leaks in ipa-join Also remove dead code in ipa-join and add initializer to a variable in ipa-getkeytab to prevent false positives in static code analysis. https://fedorahosted.org/freeipa/ticket/4651 --- ipa-client/ipa-getkeytab.c | 2 +- ipa-client/ipa-join.c | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c index 7861e4e..bb43c33 100644 --- a/ipa-client/ipa-getkeytab.c +++ b/ipa-client/ipa-getkeytab.c @@ -794,7 +794,7 @@ int main(int argc, const char *argv[]) char *password = NULL; krb5_context krbctx; krb5_ccache ccache; - krb5_principal uprinc; + krb5_principal uprinc = NULL; krb5_principal sprinc; krb5_error_code krberr; struct keys_container keys = { 0 }; diff --git a/ipa-client/ipa-join.c b/ipa-client/ipa-join.c index df33d3b..46f6457 100644 --- a/ipa-client/ipa-join.c +++ b/ipa-client/ipa-join.c @@ -463,14 +463,12 @@ static int join_ldap(const char *ipaserver, char *hostname, char ** binddn, const char *bindpw, const char *basedn, const char **princ, const char **subject, int quiet) { LDAP *ld; - char *filter = NULL; int rval = 0; char *oidresult = NULL; struct berval valrequest; struct berval *valresult = NULL; int rc, ret; char *ldap_base = NULL; - char *search_base = NULL; *binddn = NULL; *princ = NULL; @@ -542,16 +540,12 @@ join_ldap(const char *ipaserver, char *hostname, char ** binddn, const char *bin *princ = strdup(valresult->bv_val); ldap_done: - - free(filter); - free(search_base); - free(ldap_base); - if (ld != NULL) { ldap_unbind_ext(ld, NULL, NULL); } done: + free(ldap_base); if (valresult) ber_bvfree(valresult); if (oidresult) free(oidresult); return rval; @@ -815,7 +809,8 @@ unenroll_host(const char *server, const char *hostname, const char *ktname, int if (!quiet) fprintf(stderr, _("Error parsing \"%1$s\": %2$s.\n"), principal, error_message(krberr)); - return krberr; + rval = 4; + goto cleanup; } strcpy(tgs, KRB5_TGS_NAME); snprintf(tgs + strlen(tgs), sizeof(tgs) - strlen(tgs), "/%.*s", @@ -833,7 +828,8 @@ unenroll_host(const char *server, const char *hostname, const char *ktname, int if (!quiet) fprintf(stderr, _("Error obtaining initial credentials: %s.\n"), error_message(krberr)); - return krberr; + rval = 19; + goto cleanup; } krberr = krb5_cc_resolve(krbctx, "MEMORY:ipa-join", &ccache); @@ -852,7 +848,8 @@ unenroll_host(const char *server, const char *hostname, const char *ktname, int fprintf(stderr, _("Error storing creds in credential cache: %s.\n"), error_message(krberr)); - return krberr; + rval = 19; + goto cleanup; } krb5_cc_close(krbctx, ccache); ccache = NULL; @@ -914,6 +911,7 @@ cleanup: free(user_agent); if (keytab) krb5_kt_close(krbctx, keytab); + free(host); free((char *)principal); free((char *)ipaserver); if (princ) krb5_free_principal(krbctx, princ); -- 2.1.0
>From 9472a0dbc7260c8e381bb2fbcc1df1aac148c6d0 Mon Sep 17 00:00:00 2001 From: Jan Cholasta <jchol...@redhat.com> Date: Wed, 5 Nov 2014 08:59:57 +0000 Subject: [PATCH 6/6] Fix various bugs in ipap11helper Fixes a memory leak, a library handle leak and a double free. Also remove some redundant NULL checks before free to prevent false positives in static code analysis. https://fedorahosted.org/freeipa/ticket/4651 --- ipapython/ipap11helper/p11helper.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c index df5302a..038c26c 100644 --- a/ipapython/ipap11helper/p11helper.c +++ b/ipapython/ipap11helper/p11helper.c @@ -334,8 +334,7 @@ int _find_key(P11_Helper* self, CK_ATTRIBUTE_PTR template, if (tmp_objects_ptr == NULL) { *objects_count = 0; PyErr_SetString(ipap11helperError, "_find_key realloc failed"); - if (result_objects != NULL) - free(result_objects); + free(result_objects); return 0; } else { result_objects = tmp_objects_ptr; @@ -346,16 +345,14 @@ int _find_key(P11_Helper* self, CK_ATTRIBUTE_PTR template, rv = self->p11->C_FindObjects(self->session, &result_object, 1, &objectCount); if (!check_return_value(rv, "Check for duplicated key")) { - if (result_objects != NULL) - free(result_objects); + free(result_objects); return 0; } } rv = self->p11->C_FindObjectsFinal(self->session); if (!check_return_value(rv, "Find objects final")) { - if (result_objects != NULL) - free(result_objects); + free(result_objects); return 0; } @@ -499,6 +496,8 @@ static int P11_Helper_init(P11_Helper *self, PyObject *args, PyObject *kwds) { CK_C_GetFunctionList pGetFunctionList = loadLibrary(library_path, &module_handle); if (!pGetFunctionList) { + if (module_handle != NULL) + unloadLibrary(module_handle); PyErr_SetString(ipap11helperError, "Could not load the library."); return -1; } @@ -933,9 +932,7 @@ P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { if (result_list == NULL) { PyErr_SetString(ipap11helperError, "Unable to create list with results"); - if (objects != NULL) { - free(objects); - } + free(objects); return NULL; } Py_INCREF(result_list); @@ -944,13 +941,12 @@ P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { == -1) { PyErr_SetString(ipap11helperError, "Unable to add to value to result list"); - if (objects != NULL) { - free(objects); - } + free(objects); return NULL; } } + free(objects); return result_list; } @@ -1193,7 +1189,6 @@ P11_Helper_import_RSA_public_key(P11_Helper* self, CK_UTF8CHAR *label, if (rsa == NULL) { PyErr_SetString(ipap11helperError, "import_RSA_public_key: EVP_PKEY_get1_RSA error"); - free(pkey); return NULL; } @@ -1379,8 +1374,8 @@ P11_Helper_export_wrapped_key(P11_Helper* self, PyObject *args, PyObject *kwds) wrapped_key = malloc(wrapped_key_len); if (wrapped_key == NULL) { rv = CKR_HOST_MEMORY; - check_return_value(rv, "key wrapping: buffer allocation"); - return 0; + if (!check_return_value(rv, "key wrapping: buffer allocation")) + return 0; } rv = self->p11->C_WrapKey(self->session, &wrapping_mech, object_wrapping_key, object_key, wrapped_key, &wrapped_key_len); -- 2.1.0
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel