setcifsacl: fix some bugs in build_cmdline_aces

Pavel Raiskup found the following defect in setcifsacl with Coverity:

"segfault may occur also in cifs-utils-4.8.1/setcifsacl.c|644| because
of casesptr dereferencing.  When you look e.g. at the line 605, in this
time any part of 'caseptr' may be yet uninitialized and program is going
through 'goto' to freeing -> and there you are freeing the 'caseptr[i]'
address."

The analysis there seems a little off, but is basically correct. The
freeing loop counts down from the current value of i to free the
secondary allocations here.

There is one situation though where this could go badly. If the strtok
parsing near the beginning of the loop fails, then we could end up
trying to free an uninitialized pointer.

Fix this by changing the cacesptr allocation to use calloc(), and stop
trying to be clever with the freeing loop. Just have it walk the
entire array and attempt to free each slot.

Reported-by: Pavel Raiskup <praiskup@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>

1 files changed, 2 insertions, 3 deletions

diff --git a/setcifsacl.c b/setcifsacl.c
index 668ccfb..37e95e2 100644
--- a/setcifsacl.c
+++ b/setcifsacl.c
@@ -581,8 +581,7 @@ build_cmdline_aces(char **arrptr, int numcaces)
 	char *acesid, *acetype, *aceflag, *acemask;
 	struct cifs_ace **cacesptr;
 
-	cacesptr = (struct cifs_ace **)malloc(numcaces *
-				sizeof(struct cifs_aces *));
+	cacesptr = calloc(numcaces, sizeof(struct cifs_aces *));
 	if (!cacesptr) {
 		printf("%s: Error %d allocating ACE array", __func__, errno);
 		return NULL;
@@ -634,7 +633,7 @@ build_cmdline_aces(char **arrptr, int numcaces)
 	return cacesptr;
 
 build_cmdline_aces_ret:
-	for (; i >= 0; --i)
+	for (i = 0; i < numcaces; ++i)
 		free(cacesptr[i]);
 	free(cacesptr);
 	return NULL;