More fixes for static analysis issues.

Further observations from Zoltan Fridrich's static analysis of libcap.
This commit also includes a fix for something I broke with the last
round of "fixing", and a test to make sure I don't make that mistake
again.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

9 files changed, 199 insertions, 38 deletions

diff --git a/libcap/_makenames.c b/libcap/_makenames.c
index b09cf69..c0d6db4 100644
--- a/libcap/_makenames.c
+++ b/libcap/_makenames.c
@@ -45,7 +45,7 @@ int main(void)
 	if (maxcaps <= list[i].index) {
 	    maxcaps = list[i].index + 1;
 	}
-        if (list[i].index >= pointers_avail) {
+        if (pointers == NULL || list[i].index >= pointers_avail) {
 	    int was = pointers_avail * sizeof(char *);
 	    pointers_avail = 2 * list[i].index + 1;
 	    pointers = recalloc(pointers, was, pointers_avail * sizeof(char *));
diff --git a/libcap/cap_alloc.c b/libcap/cap_alloc.c
index 72317d4..df1a275 100644
--- a/libcap/cap_alloc.c
+++ b/libcap/cap_alloc.c
@@ -41,7 +41,6 @@ struct _cap_alloc_s {
 /*
  * Obtain a blank set of capabilities
  */
-
 cap_t cap_init(void)
 {
     struct _cap_alloc_s *raw_data;
@@ -222,8 +221,14 @@ int cap_free(void *data_p)
     case CAP_S_MAGIC:
 	break;
     case CAP_LAUNCH_MAGIC:
-	(void) cap_free(&data->u.launcher.iab);
-	(void) cap_free(&data->u.launcher.chroot);
+	if (cap_free(data->u.launcher.iab) != 0) {
+	    return -1;
+	}
+	data->u.launcher.iab = NULL;
+	if (cap_free(data->u.launcher.chroot) != 0) {
+	    return -1;
+	}
+	data->u.launcher.chroot = NULL;
 	break;
     default:
 	_cap_debug("don't recognize what we're supposed to liberate");
diff --git a/libcap/cap_proc.c b/libcap/cap_proc.c
index 1494f8d..eac86cb 100644
--- a/libcap/cap_proc.c
+++ b/libcap/cap_proc.c
@@ -438,13 +438,17 @@ static cap_value_t raise_cap_setpcap[] = {CAP_SETPCAP};
 
 static int _cap_set_mode(struct syscaller_s *sc, cap_mode_t flavor)
 {
-    cap_t working = cap_get_proc();
+    int ret;
     unsigned secbits = CAP_SECURED_BITS_AMBIENT;
+    cap_t working = cap_get_proc();
 
-    int ret = cap_set_flag(working, CAP_EFFECTIVE,
-			   1, raise_cap_setpcap, CAP_SET);
-    ret = ret | _cap_set_proc(sc, working);
+    if (working == NULL) {
+	_cap_debug("getting current process' capabilities failed");
+	return -1;
+    }
 
+    ret = cap_set_flag(working, CAP_EFFECTIVE, 1, raise_cap_setpcap, CAP_SET) |
+	_cap_set_proc(sc, working);
     if (ret == 0) {
 	cap_flag_t c;
 
@@ -520,7 +524,7 @@ cap_mode_t cap_get_mode(void)
 
     /* validate ambient is not set */
     int olderrno = errno;
-    int ret = 0;
+    int ret = 0, cf;
     cap_value_t c;
     for (c = 0; !ret; c++) {
 	ret = cap_get_ambient(c);
@@ -529,6 +533,7 @@ cap_mode_t cap_get_mode(void)
 	    if (c && secbits != CAP_SECURED_BITS_AMBIENT) {
 		return CAP_MODE_UNCERTAIN;
 	    }
+	    ret = 0;
 	    break;
 	}
 	if (ret) {
@@ -536,11 +541,22 @@ cap_mode_t cap_get_mode(void)
 	}
     }
 
+    /*
+     * Explore how capabilities differ from empty.
+     */
     cap_t working = cap_get_proc();
     cap_t empty = cap_init();
-    int cf = cap_compare(empty, working);
+    if (working == NULL || empty == NULL) {
+	_cap_debug("working=%p, empty=%p - need both non-NULL", working, empty);
+	ret = -1;
+    } else {
+	cf = cap_compare(empty, working);
+    }
     cap_free(empty);
     cap_free(working);
+    if (ret != 0) {
+	return CAP_MODE_UNCERTAIN;
+    }
 
     if (CAP_DIFFERS(cf, CAP_INHERITABLE)) {
 	return CAP_MODE_PURE1E;
@@ -566,6 +582,10 @@ static int _cap_setuid(struct syscaller_s *sc, uid_t uid)
 {
     const cap_value_t raise_cap_setuid[] = {CAP_SETUID};
     cap_t working = cap_get_proc();
+    if (working == NULL) {
+	return -1;
+    }
+
     (void) cap_set_flag(working, CAP_EFFECTIVE,
 			1, raise_cap_setuid, CAP_SET);
     /*
@@ -621,6 +641,10 @@ static int _cap_setgroups(struct syscaller_s *sc,
 {
     const cap_value_t raise_cap_setgid[] = {CAP_SETGID};
     cap_t working = cap_get_proc();
+    if (working == NULL) {
+	return -1;
+    }
+
     (void) cap_set_flag(working, CAP_EFFECTIVE,
 			1, raise_cap_setgid, CAP_SET);
     /*
@@ -718,10 +742,9 @@ cap_iab_t cap_iab_get_proc(void)
  */
 static int _cap_iab_set_proc(struct syscaller_s *sc, cap_iab_t iab)
 {
-    int ret, i;
-    cap_t working, temp = cap_get_proc();
+    int ret, i, raising = 0;
     cap_value_t c;
-    int raising = 0;
+    cap_t working, temp = cap_get_proc();
 
     if (temp == NULL) {
 	return -1;
@@ -737,6 +760,10 @@ static int _cap_iab_set_proc(struct syscaller_s *sc, cap_iab_t iab)
     }
 
     working = cap_dup(temp);
+    if (working == NULL) {
+	ret = -1;
+	goto defer;
+    }
     if (raising) {
 	ret = cap_set_flag(working, CAP_EFFECTIVE,
 			   1, raise_cap_setpcap, CAP_SET);
@@ -861,6 +888,10 @@ static int _cap_chroot(struct syscaller_s *sc, const char *root)
 {
     const cap_value_t raise_cap_sys_chroot[] = {CAP_SYS_CHROOT};
     cap_t working = cap_get_proc();
+    if (working == NULL) {
+	return -1;
+    }
+
     (void) cap_set_flag(working, CAP_EFFECTIVE,
 			1, raise_cap_sys_chroot, CAP_SET);
     int ret = _cap_set_proc(sc, working);
diff --git a/libcap/cap_test.c b/libcap/cap_test.c
index e8eb647..41192b7 100644
--- a/libcap/cap_test.c
+++ b/libcap/cap_test.c
@@ -5,11 +5,13 @@
 
 static cap_value_t top;
 
-static int cf(cap_value_t x) {
+static int cf(cap_value_t x)
+{
     return top - x - 1;
 }
 
-static int test_cap_bits(void) {
+static int test_cap_bits(void)
+{
     static cap_value_t vs[] = {
 	5, 6, 11, 12, 15, 16, 17, 38, 41, 63, 64, __CAP_MAXBITS+3, 0, -1
     };
@@ -35,10 +37,12 @@ static int test_cap_bits(void) {
     return failed;
 }
 
-static int test_cap_flags(void) {
+static int test_cap_flags(void)
+{
     cap_t c, d;
     cap_flag_t f = CAP_INHERITABLE, t;
     cap_value_t v;
+    int retval = 0;
 
     c = cap_init();
     if (c == NULL) {
@@ -49,7 +53,8 @@ static int test_cap_flags(void) {
     for (v = 0; v < __CAP_MAXBITS; v += 3) {
 	if (cap_set_flag(c, CAP_INHERITABLE, 1, &v, CAP_SET)) {
 	    printf("unable to set inheritable bit %d\n", v);
-	    return -1;
+	    retval = -1;
+	    goto drop_c;
 	}
     }
 
@@ -57,24 +62,36 @@ static int test_cap_flags(void) {
     for (t = CAP_EFFECTIVE; t <= CAP_INHERITABLE; t++) {
 	if (cap_fill(c, t, f)) {
 	    printf("cap_fill failed %d -> %d\n", f, t);
-	    return -1;
+	    retval = -1;
+	    goto drop_d;
 	}
 	if (cap_clear_flag(c, f)) {
 	    printf("cap_fill unable to clear flag %d\n", f);
-	    return -1;
+	    retval = -1;
+	    goto drop_d;
 	}
 	f = t;
     }
     if (cap_compare(c, d)) {
 	printf("permuted cap_fill()ing failed to perform net no-op\n");
-	return -1;
+	retval = -1;
+    }
+
+drop_d:
+    if (cap_free(d) != 0) {
+	perror("failed to free d");
+	retval = -1;
+    }
+drop_c:
+    if (cap_free(c) != 0) {
+	perror("failed to free c");
+	retval = -1;
     }
-    cap_free(d);
-    cap_free(c);
-    return 0;
+    return retval;
 }
 
-static int test_short_bits(void) {
+static int test_short_bits(void)
+{
     int result = 0;
     char *tmp;
     int n = asprintf(&tmp, "%d", __CAP_MAXBITS);
@@ -90,12 +107,87 @@ static int test_short_bits(void) {
     return result;
 }
 
+static int noop(void *data)
+{
+    return -1;
+}
+
+static int test_alloc(void)
+{
+    int retval = 0;
+    cap_t c;
+    cap_iab_t iab;
+    cap_launch_t launcher;
+
+    c = cap_init();
+    if (c == NULL) {
+	perror("failed to allocate a cap_t");
+	return -1;
+    }
+
+    iab = cap_iab_init();
+    if (iab == NULL) {
+	perror("failed to allocate a cap_iab_t");
+	retval = -1;
+	goto drop_c;
+    }
+
+    launcher = cap_func_launcher(noop);
+    if (launcher == NULL) {
+	perror("failde to allocate a launcher");
+	retval = -1;
+	goto drop_iab;
+    }
+
+    cap_launcher_set_chroot(launcher, "/tmp");
+    if (cap_launcher_set_iab(launcher, iab) != NULL) {
+	printf("unable to replace iab in launcher\n");
+	retval = -1;
+	goto drop_iab;
+    }
+
+    iab = cap_launcher_set_iab(launcher, cap_iab_init());
+    if (iab == NULL) {
+	printf("unable to recover iab in launcher\n");
+	retval = -1;
+	goto drop_launcher;
+    }
+
+drop_launcher:
+    if (cap_free(launcher)) {
+	perror("failed to free launcher");
+	retval = -1;
+    }
+
+drop_iab:
+    if (!cap_free(2+(__u32 *) iab)) {
+	printf("unable to recognize bad cap_iab_t pointer\n");
+	retval = -1;
+    }
+    if (cap_free(iab)) {
+	perror("failed to free iab");
+	retval = -1;
+    }
+
+drop_c:
+    if (!cap_free(1+(__u32 *)c)) {
+	printf("unable to recognize bad cap_t pointer\n");
+	retval = -1;
+    }
+    if (cap_free(c)) {
+	perror("failed to free c");
+	retval = -1;
+    }
+    return retval;
+}
+
 int main(int argc, char **argv) {
     int result = 0;
 
     result = test_cap_bits() | result;
     result = test_cap_flags() | result;
     result = test_short_bits() | result;
+    result = test_alloc() | result;
 
     if (result) {
 	printf("cap_test FAILED\n");
diff --git a/pam_cap/execable.c b/pam_cap/execable.c
index 0bf42d3..05f18d8 100644
--- a/pam_cap/execable.c
+++ b/pam_cap/execable.c
@@ -36,7 +36,7 @@ SO_MAIN(int argc, char **argv)
 	return;
     }
 
-    if (argc > 2 || strcmp(argv[1], "--help")) {
+    if (argc > 2 || argv[1] == NULL || strcmp(argv[1], "--help")) {
 	printf("\n%s only supports the optional argument --help\n", cmd);
 	exit(1);
     }
diff --git a/progs/capsh.c b/progs/capsh.c
index 42d9064..763c08d 100644
--- a/progs/capsh.c
+++ b/progs/capsh.c
@@ -89,6 +89,10 @@ static void display_current(void)
     char *text;
 
     all = cap_get_proc();
+    if (all == NULL) {
+	perror("failed to get process capabilities");
+	exit(1);
+    }
     text = cap_to_text(all, NULL);
     printf("Current: %s\n", text);
     cap_free(text);
@@ -922,6 +926,10 @@ int main(int argc, char *argv[], char *envp[])
 		exit(1);
 	    }
 	    orig = cap_get_proc();
+	    if (orig == NULL) {
+		perror("failed to get process capabilities");
+		exit(1);
+	    }
 	    if (cap_get_flag(orig, cap, CAP_PERMITTED, &enabled) || !enabled) {
 		fprintf(stderr, "cap[%s] not permitted\n", argv[i]+8);
 		exit(1);
@@ -938,6 +946,10 @@ int main(int argc, char *argv[], char *envp[])
 		exit(1);
 	    }
 	    orig = cap_get_proc();
+	    if (orig == NULL) {
+		perror("failed to get process capabilities");
+		exit(1);
+	    }
 	    if (cap_get_flag(orig, cap, CAP_INHERITABLE, &enabled)
 		|| !enabled) {
 		fprintf(stderr, "cap[%s] not inheritable\n", argv[i]+8);
diff --git a/progs/setcap.c b/progs/setcap.c
index 066e47f..f8be53a 100644
--- a/progs/setcap.c
+++ b/progs/setcap.c
@@ -85,9 +85,12 @@ int main(int argc, char **argv)
 		" (old libcap?)\n");
     }
 
+    cap_t cap_d = NULL;
     while (--argc > 0) {
 	const char *text;
-	cap_t cap_d;
+
+	cap_free(cap_d);
+	cap_d = NULL;
 
 	if (!strcmp(*++argv, "-q")) {
 	    quiet = 1;
@@ -109,7 +112,8 @@ int main(int argc, char **argv)
 	}
 	if (!strcmp(*argv, "-n")) {
 	    if (argc < 2) {
-		fprintf(stderr, "usage: .. -n <rootid> .. - rootid!=0 file caps");
+		fprintf(stderr,
+			"usage: .. -n <rootid> .. - rootid!=0 file caps");
 		exit(1);
 	    }
 	    --argc;
@@ -122,6 +126,7 @@ int main(int argc, char **argv)
 	}
 
 	if (!strcmp(*argv, "-r")) {
+	    cap_free(cap_d);
 	    cap_d = NULL;
 	} else {
 	    if (!strcmp(*argv,"-")) {
@@ -144,11 +149,9 @@ int main(int argc, char **argv)
 	    }
 #ifdef DEBUG
 	    {
-		ssize_t length;
-		const char *result;
-
-		result = cap_to_text(cap_d, &length);
+		char *result = cap_to_text(cap_d, NULL);
 		fprintf(stderr, "caps set to: [%s]\n", result);
+		cap_free(result)
 	    }
 #endif
 	}
@@ -163,12 +166,16 @@ int main(int argc, char **argv)
 	    int cmp;
 
 	    if (cap_d == NULL) {
-		cap_d = cap_from_text("=");
+		cap_d = cap_init();
+		if (cap_d == NULL) {
+		    perror("unable to obtain empty capability");
+		    exit(1);
+		}
 	    }
 
 	    cap_on_file = cap_get_file(*++argv);
 	    if (cap_on_file == NULL) {
-		cap_on_file = cap_from_text("=");
+		cap_on_file = cap_init();
 		if (cap_on_file == NULL) {
 		    perror("unable to use missing capability");
 		    exit(1);
@@ -264,9 +271,9 @@ int main(int argc, char **argv)
 		}
 	    }
 	}
-	if (cap_d) {
-	    cap_free(cap_d);
-	}
+    }
+    if (cap_d) {
+	cap_free(cap_d);
     }
 
     exit(0);
diff --git a/tests/libcap_launch_test.c b/tests/libcap_launch_test.c
index d1b3d40..343e389 100644
--- a/tests/libcap_launch_test.c
+++ b/tests/libcap_launch_test.c
@@ -120,6 +120,10 @@ int main(int argc, char **argv) {
     };
 
     cap_t orig = cap_get_proc();
+    if (orig == NULL) {
+	perror("failed to get process capabilities");
+	exit(1);
+    }
 
     int success = 1, i;
     for (i=0; vs[i].pass_on != NO_MORE; i++) {
@@ -201,6 +205,10 @@ int main(int argc, char **argv) {
     }
 
     cap_t final = cap_get_proc();
+    if (final == NULL) {
+	perror("unable to get final capabilities");
+	exit(1);
+    }
     if (cap_compare(orig, final)) {
 	char *was = cap_to_text(orig, NULL);
 	char *is = cap_to_text(final, NULL);
diff --git a/tests/libcap_psx_test.c b/tests/libcap_psx_test.c
index e473126..9ef8cac 100644
--- a/tests/libcap_psx_test.c
+++ b/tests/libcap_psx_test.c
@@ -21,7 +21,10 @@ static void *thread_fork_exit(void *data) {
 	exit(1);
     }
     if (pid == 0) {
-	cap_set_proc(start);
+	if (cap_set_proc(start)) {
+	    perror("setting empty caps failed");
+	    exit(1);
+	}
 	exit(0);
     }
     int res;
@@ -51,7 +54,10 @@ int main(int argc, char **argv) {
     for (i = 0; i < 10; i++) {
 	printf(".");     /* because of fork, this may print double */
 	fflush(stdout);  /* try to limit the above effect */
-	cap_set_proc(start);
+	if (cap_set_proc(start)) {
+	    perror("failed to set proc");
+	    exit(1);
+	}
 	usleep(1000);
     }
     printf(" PASSED\n");