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

Reply via email to