Fix release static analysis findings

safe_exec under-allocated argv by not reserving a slot for execve's terminating NULL, and several call sites omitted the required NULL sentinel, letting the varargs reader run past the provided arguments. The argv array now has a terminator slot and every safe_exec call passes an explicit sentinel.

GSS ACK handling in auditd-listen allocated utok.value and copied into it without checking for allocation failure. The GSS path now logs and drops the ACK on ENOMEM before touching the buffer.

auparse file source setup allocated source_list entries without checking the list or filename strdup allocations, leaving partial lists on failure. The setup now calloc's NULL-terminated lists, frees partial lists through a shared helper, and reuses that helper during destroy and init failure cleanup.

audisp-remote's GSS connection setup left the connected socket open if credential negotiation failed after connect. The failure path now calls stop_sock() before returning the permanent error so the socket and GSS/KRB state are torn down consistently.

The IDS session model treated the account string as transferred to new_session even though new_session could fail before taking ownership, and it could duplicate a NULL interpreted field. The caller now only duplicates non-NULL fields and always frees its local copy, while new_session makes its own stored copy and frees it if insertion fails.

Author: stevegrubb

audisp/plugins/ids/model_bad_event.c

@@ -44,12 +44,16 @@ static void start_session(auparse_state_t *au, struct ids_conf *config)
 		a = -1;
 
 	int service_acct = 0;
-	const char *acct = NULL;
+	char *acct = NULL;
 	const char *atype = auparse_normalize_subject_kind(au);
 	if (atype && strncmp(atype, "service", 7) == 0)
 		service_acct = 1;
-	if (auparse_normalize_subject_primary(au) == 1)
-		acct = strdup(auparse_interpret_field(au));
+	if (auparse_normalize_subject_primary(au) == 1) {
+		const char *field = auparse_interpret_field(au);
+
+		if (field)
+			acct = strdup(field);
+	}
 
 	// Have we seen this endpoint before?
 	origin_data_t *o = find_origin(a);
@@ -60,7 +64,8 @@ static void start_session(auparse_state_t *au, struct ids_conf *config)
 
 	// Is this login a service account?
 	if (service_acct && !config->option_service_login_allowed) {
-		my_printf("bad_service_login_origin: %s", acct);
+		my_printf("bad_service_login_origin: %s",
+				acct ? acct : "?");
 		bad_service_login_origin(o, config, acct);
 	}
 
@@ -88,15 +93,14 @@ static void start_session(auparse_state_t *au, struct ids_conf *config)
 	if (auparse_normalize_session(au) == 1) {
 		unsigned int s = auparse_get_field_int(au);
 		if (s != UNSET) {
-			// new_session takes custody of acct
+			// new_session copies acct before storing it.
 			new_session(s, a, acct);
-			acct = NULL;
 		// otherwise we have a strange daemon login
 		} else if (debug)
 		    my_printf("start_session: can't find session in serial %s",
 				auparse_get_type_name(au));
 	}
-	free((void *)acct);
+	free(acct);
 }
 
 static void end_session(auparse_state_t *au)
@@ -110,6 +114,7 @@ static void end_session(auparse_state_t *au)
 	}
 }
 
+
 /* This function receives a single complete event from the auparse library. */
 void process_bad_event_model(auparse_state_t *au,
 	struct ids_conf *config)
@@ -165,4 +170,3 @@ void process_bad_event_model(auparse_state_t *au,
 		}
 	}
 }
-

audisp/plugins/ids/reactions.c

@@ -92,16 +92,15 @@ static int safe_exec(const char *exe, ...)
 	va_start(ap, exe);
 	for (i = 1; va_arg(ap, char *) != NULL; i++);
 	va_end(ap);
-	argv = alloca(i * sizeof(char *));
+	argv = alloca((i + 1) * sizeof(char *));
 
 	va_start(ap, exe);
 	argv[0] = (char *) exe;
 	for (i = 1; (argv[i] = (char *) va_arg(ap, char *)) != NULL; i++);
 	va_end(ap);
-	argv[i] = NULL;
 
 	execve(exe, argv, NULL);
-        syslog(LOG_ALERT, "Audit IDS failed to exec %s", exe);
+	syslog(LOG_ALERT, "Audit IDS failed to exec %s", exe);
 	exit(1);
 }
 
@@ -205,36 +204,36 @@ int restricted_role(const char *acct)
 
 	// Restrict to guest user
 	rc = safe_exec("/usr/sbin/semanage", "login", "-m", "-s",
-		"guest_u", acct);
+		"guest_u", acct, NULL);
 	if (rc)
 		return rc;
 
 	// Need to force a logout of all sessions for the user
-	return safe_exec("/usr/bin/killall", "--user", acct);
+	return safe_exec("/usr/bin/killall", "--user", acct, NULL);
 }
 
 int force_password_reset(const char *acct)
 {
 	if (verify_acct(acct))
 		return 1;
 
-	return safe_exec("/usr/bin/chage", "-d", "0", acct);
+	return safe_exec("/usr/bin/chage", "-d", "0", acct, NULL);
 }
 
 int lock_account(const char *acct)
 {
 	if (verify_acct(acct))
 		return 1;
 
-	return safe_exec("/usr/bin/passwd", "-l", acct);
+	return safe_exec("/usr/bin/passwd", "-l", acct, NULL);
 }
 
 int unlock_account(const char *acct)
 {
 	if (verify_acct(acct))
 		return 1;
 
-	return safe_exec("/usr/bin/passwd", "-u", acct);
+	return safe_exec("/usr/bin/passwd", "-u", acct, NULL);
 }
 
 int lock_account_timed(const char *acct, unsigned long length)
@@ -327,17 +326,17 @@ int unblock_ip_address(const char *addr)
 
 int system_reboot(void)
 {
-	return safe_exec("/sbin/init", "6");
+	return safe_exec("/sbin/init", "6", NULL);
 }
 
 int system_single_user(void)
 {
-	return safe_exec("/sbin/init", "1");
+	return safe_exec("/sbin/init", "1", NULL);
 }
 
 int system_halt(void)
 {
-	return safe_exec("/sbin/init", "0");
+	return safe_exec("/sbin/init", "0", NULL);
 }
 
 /*

audisp/plugins/ids/session.c

@@ -85,17 +85,36 @@ static void destroy_session(void)
 	cur = NULL;
 }
 
+/*
+ * new_session - create a session
+ * Args:
+ *   s    - session id
+ *   o    - origin address
+ *   acct - account string to copy into the session
+ * Rtns:
+ *   void
+ */
 void new_session(unsigned int s, unsigned int o, const char *acct)
 {
 	session_data_t *tmp = malloc(sizeof(session_data_t));
-	if (tmp) {
-		tmp->session = s;
-		tmp->score = 0;
-		tmp->killed = 0;
-		tmp->origin = o;
-		tmp->acct = acct ? acct : strdup("");
-		add_session(tmp);
+
+	if (tmp == NULL)
+		return;
+
+	tmp->acct = acct ? strdup(acct) : strdup("");
+	if (tmp->acct == NULL) {
+		free(tmp);
+		return;
 	}
+
+	tmp->session = s;
+	tmp->score = 0;
+	tmp->killed = 0;
+	tmp->origin = o;
+	if (add_session(tmp))
+		return;
+
+	free_session(tmp);
 }
 
 void destroy_sessions(void)
@@ -194,4 +213,3 @@ void add_to_score_session(session_data_t *s, unsigned int adj)
 	if (debug)
 		my_printf("session score: %u", s->score);
 }
-

audisp/plugins/remote/audisp-remote.c

@@ -2074,6 +2074,7 @@ static int init_sock(void)
 #ifdef USE_GSSAPI
 	if (USE_GSS) {
 		if (negotiate_credentials()) {
+			stop_sock();
 			rc = ET_PERMANENT;
 			goto out;
 		}

auparse/auparse.c

@@ -73,6 +73,25 @@ static int access_ok(const char *filename)
 #endif
 }
 
+/*
+ * free_source_list - release a NULL-terminated source file list
+ * Args:
+ *   list - array of allocated file names
+ * Rtns:
+ *   void
+ */
+static void free_source_list(char **list)
+{
+	int n = 0;
+
+	if (list == NULL)
+		return;
+
+	while (list[n])
+		free(list[n++]);
+	free(list);
+}
+
 static int setup_log_file_array(auparse_state_t *au)
 {
         struct daemon_conf config;
@@ -490,11 +509,14 @@ auparse_state_t *auparse_init(ausource_t source, const void *b)
 				goto bad_exit;
 			if (access_ok(b))
 				goto bad_exit;
-			tmp = malloc(2*sizeof(char *));
+			tmp = calloc(2, sizeof(char *));
 			if (tmp == NULL)
 				goto bad_exit;
 			tmp[0] = strdup(b);
-			tmp[1] = NULL;
+			if (tmp[0] == NULL) {
+				free_source_list(tmp);
+				goto bad_exit;
+			}
 			au->source_list = tmp;
 			break;
 		case AUSOURCE_FILE_ARRAY:
@@ -506,10 +528,16 @@ auparse_state_t *auparse_init(ausource_t source, const void *b)
 					goto bad_exit;
 				n++;
 			}
-			tmp = malloc((n+1)*sizeof(char *));
-			for (i=0; i<n; i++)
+			tmp = calloc(n+1, sizeof(char *));
+			if (tmp == NULL)
+				goto bad_exit;
+			for (i=0; i<n; i++) {
 				tmp[i] = strdup(bb[i]);
-			tmp[n] = NULL;
+				if (tmp[i] == NULL) {
+					free_source_list(tmp);
+					goto bad_exit;
+				}
+			}
 			au->source_list = tmp;
 			break;
 		case AUSOURCE_BUFFER:
@@ -578,6 +606,7 @@ auparse_state_t *auparse_init(ausource_t source, const void *b)
 
 	return au;
 bad_exit:
+	free_source_list(au->source_list);
 	databuf_free(&au->databuf);
 	/* Free list of events list (au_lo) structure */
 	au_lol_clear(au->au_lo, 0);
@@ -1103,13 +1132,8 @@ static void auparse_destroy_common(auparse_state_t *au)
 	if (au == NULL)
 		return;
 
-	if (au->source_list) {
-		int n = 0;
-		while (au->source_list[n])
-			free(au->source_list[n++]);
-		free(au->source_list);
-		au->source_list = NULL;
-	}
+	free_source_list(au->source_list);
+	au->source_list = NULL;
 
 	au->next_buf = NULL;
         free(au->cur_buf);

src/auditd-listen.c

@@ -1083,6 +1083,11 @@ static void client_ack(void *ack_data, const unsigned char *header,
 		mlen = strlen(msg);
 		utok.length = AUDIT_RMW_HEADER_SIZE + mlen;
 		utok.value = malloc(utok.length + 1);
+		if (utok.value == NULL) {
+			audit_msg(LOG_ERR, "GSS-API ACK to %s lost: "
+				"out of memory", sockaddr_to_addr(&io->addr));
+			return;
+		}
 
 		memcpy(utok.value, header, AUDIT_RMW_HEADER_SIZE);
 		memcpy(utok.value+AUDIT_RMW_HEADER_SIZE, msg, mlen);