[Freeipa-devel] [PATCH] Coverity fixes for slapi-nis

[Alexander Bokovoy]

Hi Nalin,

attached please find two patches that fix minor Coverity issues.

The first patch is for issue 11937 which is a false positive but caught
up wrong use of the helper method -- the method map_data_set_entry()
passes key and value length arguments through to map_data_save_list()
which expects them to be arrays but we pass pointer to the variable.
Luckily, in our case map_data_save_list() never goes beyond element 0 of
the array so the fix is mostly cosmetic.

The second fix is in PAM wrapper in the tests and minor too -- we would
leak a memory if PAM wrapper wasn't called under wrapping condition.

The same patches are in my Fedora people slapi-nis tree, branch
'coverity':
http://fedorapeople.org/cgit/abbra/public_git/slapi-nis.git/log/?h=coverity


-- 
/ Alexander Bokovoy
-------------- next part --------------
>From ec00422ceeabc1296031675ff0cbd559cbd23806 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Mon, 2 Sep 2013 15:58:09 +0300
Subject: [PATCH 1/2] Coverity#11937: use proper structure to pass to
 map_data_set_entry()

map_data_set_entry() passes pointers to the lengths of the key and the value
to map_data_save_list() which interpretes them as arrays of integers.
---
 src/back-sch.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/back-sch.c b/src/back-sch.c
index c33e708..9d14a7e 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -394,7 +394,7 @@ backend_set_entry_from(Slapi_PBlock *pb, enum backend_entry_source source,
 	const char *hexchars = "0123456789ABCDEF";
 	char *rdn, *ndn, *ldif, *plugin_id, *keys[2], *values[2], **ava, *p, *q;
 	char *usn, *attr, *val;
-	unsigned int rdn_len, value_len, *ava_lens;
+	unsigned int rdn_len[2], value_len[2], *ava_lens;
 	const char *rdnstr;
 	int len, i, j, k, count;
 	Slapi_Entry *entry;
@@ -421,7 +421,7 @@ backend_set_entry_from(Slapi_PBlock *pb, enum backend_entry_source source,
 			      &data->common.inref_attrs,
 			      &data->common.ref_attr_list,
 			      &data->common.inref_attr_list,
-			      &rdn_len);
+			      rdn_len);
 	if ((rdn == NULL) || (strlen(rdn) == 0) || (strchr(rdn, '=') == NULL)) {
 		slapi_log_error(SLAPI_LOG_FATAL, plugin_id,
 				"no RDN for %s, unsetting domain/map/id "
@@ -601,14 +601,16 @@ backend_set_entry_from(Slapi_PBlock *pb, enum backend_entry_source source,
 				rdn, ndn, slapi_entry_get_ndn(entry));
 		keys[0] = (char *) rdnstr;
 		keys[1] = NULL;
-		rdn_len = strlen(rdnstr);
+		rdn_len[0] = strlen(rdnstr);
+		rdn_len[1] = -1;
 		values[0] = (char *) slapi_entry_get_ndn(entry);
 		values[1] = NULL;
-		value_len = -1;
+		value_len[0] = -1;
+		value_len[1] = -1;
 		map_data_set_entry(data->common.state,
 				   data->common.group, data->common.set, ndn,
-				   &rdn_len, keys,
-				   &value_len, values,
+				   rdn_len, keys,
+				   value_len, values,
 				   backend_entry_make_entry_data(source, e_dn,
 								 entry),
 				   backend_entry_free_entry_data);
-- 
1.8.3.1

-------------- next part --------------
>From b58a7192b683fe7ab4c8c3d95d73ed4223f7247a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Mon, 2 Sep 2013 16:39:30 +0300
Subject: [PATCH 2/2] Coverity#11940: do not leak memory in the pam wrapper
 test

---
 tests/wrap-pam.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/wrap-pam.c b/tests/wrap-pam.c
index dc92287..45fa0fc 100644
--- a/tests/wrap-pam.c
+++ b/tests/wrap-pam.c
@@ -111,14 +111,16 @@ pam_start(const char *service_name, const char *user,
 	char buf[LINE_MAX], *p, *q;
 	pam_handle_t *ret;
 
+	if (getenv("WRAPPERS_PAM_CREDS") == NULL) {
+		return PAM_ABORT;
+	}
+
 	ret = calloc(1, sizeof(*ret));
 	if (ret == NULL) {
 		return PAM_BUF_ERR;
 	}
 	ret->conv = *pam_conversation;
-	if (getenv("WRAPPERS_PAM_CREDS") == NULL) {
-		return PAM_ABORT;
-	}
+
 	fp = fopen(getenv("WRAPPERS_PAM_CREDS"), "r");
 	if (fp == NULL) {
 		free(ret);
-- 
1.8.3.1