Fix sporadic xrdp-sesman crash on session initiation

Fix a number of memory leaks
Fix access to freed memory
Fix invalid function return values

raptorsmiface/libraptorsmiface.c

@@ -53,10 +53,10 @@
 #define RAPTORSMIFACE_CFG_DATABASE_USER		"User"
 #define RAPTORSMIFACE_CFG_DATABASE_PASSWORD	"Password"
 
-char *server = "localhost";
+char *server = NULL;
-char *user = "remotelab";
+char *user = NULL;
-char *password = "";
+char *password = NULL;
-char *database = "remotelab_sm";
+char *database = NULL;
 
 void dprint(const char *fmt, ...)
 {
@@ -84,6 +84,12 @@ void raptorsmiface_config_read_database(int file, struct list* param_n, struct l
 	char* buf;
 	char* temp_buf;
 
+	// Set defaults
+	if (!server) server = strdup("localhost");
+	if (!user) user = strdup("remotelab");
+	if (!password) password = strdup("");
+	if (!database) database = strdup("remotelab_sm");
+
 	list_clear(param_v);
 	list_clear(param_n);
 
@@ -91,16 +97,20 @@ void raptorsmiface_config_read_database(int file, struct list* param_n, struct l
 	for (i = 0; i < param_n->count; i++) {
 		buf = (char*)list_get_item(param_n, i);
 		if (0 == g_strcasecmp(buf, RAPTORSMIFACE_CFG_DATABASE_SERVER)) {
-			server = g_strdup((char*)list_get_item(param_v, i));
+			free(server);
+			server = strdup((char*)list_get_item(param_v, i));
 		}
 		if (0 == g_strcasecmp(buf, RAPTORSMIFACE_CFG_DATABASE_NAME)) {
-			database = g_strdup((char*)list_get_item(param_v, i));
+			free(database);
+			database = strdup((char*)list_get_item(param_v, i));
 		}
 		if (0 == g_strcasecmp(buf, RAPTORSMIFACE_CFG_DATABASE_USER)) {
-			user = g_strdup((char*)list_get_item(param_v, i));
+			free(user);
+			user = strdup((char*)list_get_item(param_v, i));
 		}
 		if (0 == g_strcasecmp(buf, RAPTORSMIFACE_CFG_DATABASE_PASSWORD)) {
-			password = g_strdup((char*)list_get_item(param_v, i));
+			free(password);
+			password = strdup((char*)list_get_item(param_v, i));
 		}
 	}
 
@@ -172,13 +182,13 @@ char* get_group_for_user(char* username) {
 	struct passwd* pwd;
 	pwd = getpwnam(username);
 	if (!pwd) {
-		return true;
+		return NULL;
 	}
 	gid_t groupid = pwd->pw_gid;
 	struct group* primarygroup;
 	primarygroup = getgrgid(groupid);
 	if (!primarygroup) {
-		return true;
+		return NULL;
 	}
 
 	return strdup(primarygroup->gr_name);
@@ -231,6 +241,7 @@ char raptor_sm_deallocate_session(char* username) {
 		// Open the command for reading
 		fp = popen(command_string, "r");
 		if (fp == NULL) {
+			free(ip);
 			mysql_close(conn);
 			return -1;
 		}
@@ -333,6 +344,9 @@ char* raptor_sm_allocate_session(char* username) {
 
 				// Get group for user
 				char* groupname = get_group_for_user(username);
+				if (!groupname) {
+					return strdup("ERROR");
+				}
 				char* safe_groupname = get_mysql_escaped_string(conn, groupname);
 				free(groupname);
 				// Get the list of allowed nodes for this group
@@ -390,7 +404,7 @@ char* raptor_sm_allocate_session(char* username) {
 					mysql_free_result(res);
 					mysql_free_result(svr_res);
 					mysql_free_result(per_res);
-	
+
 					if (strcmp(bestserver, "") != 0) {
 						// Insert new information into the sessions database and set status to ALLOCATED
 						char* safe_servername = get_mysql_escaped_string(conn, bestserver);
@@ -445,18 +459,18 @@ char* raptor_sm_get_ip_for_hostname(char* hostname, char* error) {
 	struct addrinfo hints, *res;
 	struct in_addr addr;
 	int err;
-	
+
 	memset(&hints, 0, sizeof(hints));
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_family = AF_INET;
-	
+
 	if ((err = getaddrinfo(hostname, NULL, &hints, &res)) != 0) {
 		if (error) *error = 1;
 		return strdup("");
 	}
-	
+
 	addr.s_addr = ((struct sockaddr_in *)(res->ai_addr))->sin_addr.s_addr;
-	
+
 	char* ret = strdup(inet_ntoa(addr));
 	freeaddrinfo(res);
 	if (error) *error = 0;
@@ -565,12 +579,15 @@ bool raptor_sm_sesslimit_reached(char* username) {
 
 	// Get group for user
 	char* groupname = get_group_for_user(username);
+	if (!groupname) {
+		return true;
+	}
 	char* safe_groupname = get_mysql_escaped_string(conn, groupname);
-	free(groupname);
 	asprintf(&query, "SELECT sesslimit FROM groups WHERE groupname='%s'", safe_groupname);
 	free(safe_groupname);
 	if (mysql_query_internal(conn, query)) {
 		// Server error
+		free(groupname);
 		free(query);
 		mysql_close(conn);
 		return true;
@@ -590,6 +607,7 @@ bool raptor_sm_sesslimit_reached(char* username) {
 	asprintf(&query, "SELECT username FROM sessions WHERE state<>'%d'", SM_STATUS_ALLOCATED);
 	if (mysql_query_internal(conn, query)) {
 		// Server error
+		free(groupname);
 		free(query);
 		mysql_close(conn);
 		return true;
@@ -600,6 +618,10 @@ bool raptor_sm_sesslimit_reached(char* username) {
 		while ((row = mysql_fetch_row(res)) != NULL) {
 			if (row[0]) {
 				char* test_groupname = get_group_for_user(row[0]);
+				if (!test_groupname) {
+					free(groupname);
+					return true;
+				}
 				if (strcmp(groupname, test_groupname) == 0) {
 					sesscount++;
 				}
@@ -609,14 +631,17 @@ bool raptor_sm_sesslimit_reached(char* username) {
 		mysql_free_result(res);
 
 		if (sesscount < sesslimit) {
+			free(groupname);
 			mysql_close(conn);
 			return false;
 		}
+		free(groupname);
 		mysql_close(conn);
 		return true;
 	}
 
 	// We should never end up here!
+	free(groupname);
 	mysql_close(conn);
 	return true;
 }
@@ -672,6 +697,7 @@ pid_t raptor_sm_run_remote_server(char* username, char *const argv[], char* dbfi
 					// Open the command for reading
 					fp = popen(command_string, "r");
 					if (fp == NULL) {
+						free(ip);
 						mysql_close(conn);
 						return -1;
 					}
@@ -734,6 +760,7 @@ pid_t raptor_sm_run_remote_server(char* username, char *const argv[], char* dbfi
 	}
 	dprint("Running command %s...\n\r", command_string);
 	free(origstr);
+	free(ipaddr);
 
 	FILE *fp;
 	char output[1024];
@@ -798,7 +825,7 @@ pid_t raptor_sm_get_pid_for_username(char* username, char* dbfield) {
 	return -3;
 }
 
-char* raptor_sm_server_started(char* username, pid_t pid, int display, char* dbfield) {
+int raptor_sm_server_started(char* username, pid_t pid, int display, char* dbfield) {
 	MYSQL_RES *res;
 	MYSQL_ROW row;
 	char* query;
@@ -843,7 +870,7 @@ char* raptor_sm_server_started(char* username, pid_t pid, int display, char* dbf
 	}
 }
 
-char* raptor_sm_wm_started(char* username, pid_t pid, char* dbfield) {
+int raptor_sm_wm_started(char* username, pid_t pid, char* dbfield) {
 	MYSQL_RES *res;
 	MYSQL_ROW row;
 	char* query;
@@ -919,7 +946,7 @@ char* raptor_sm_get_username_for_display_and_hostname(int display, char* hostnam
 
 	MYSQL *conn = connect_if_needed();
 	if (!conn) {
-		return -1;
+		return strdup("");
 	}
 
 	char* safe_hostname = get_mysql_escaped_string(conn, hostname);
@@ -967,7 +994,7 @@ void raptor_sm_session_terminated(char* username) {
 	raptor_sm_deallocate_session(username);
 }
 
-void raptor_sm_wm_terminated(char* username) {
+int raptor_sm_wm_terminated(char* username) {
 	MYSQL_RES *res;
 	MYSQL_ROW row;
 	char* query;
@@ -1104,7 +1131,7 @@ void raptor_sm_terminate_server(char* username) {
 	}
 }
 
-void raptor_sm_stats_report_server_start(char* hostname) {
+int raptor_sm_stats_report_server_start(char* hostname) {
 	MYSQL_RES *res;
 	MYSQL_ROW row;
 	char* query;
@@ -1125,9 +1152,11 @@ void raptor_sm_stats_report_server_start(char* hostname) {
 	}
 	free(query);
 	mysql_close(conn);
+
+	return 0;
 }
 
-void raptor_sm_stats_report_server_stop(char* hostname) {
+int raptor_sm_stats_report_server_stop(char* hostname) {
 	MYSQL_RES *res;
 	MYSQL_ROW row;
 	char* query;
@@ -1148,4 +1177,6 @@ void raptor_sm_stats_report_server_stop(char* hostname) {
 	}
 	free(query);
 	mysql_close(conn);
+
+	return 0;
 }

raptorsmiface/libraptorsmiface.h

@@ -47,18 +47,18 @@ char* raptor_sm_get_hostname_for_username(char* username, bool create);
 char* raptor_sm_get_ip_for_username(char* username, bool create);
 pid_t raptor_sm_run_remote_server(char* username, char *const argv[], char* dbfield, int display);
 pid_t raptor_sm_get_pid_for_username(char* username, char* dbfield);
-char* raptor_sm_server_started(char* username, pid_t pid, int display, char* dbfield);
+int   raptor_sm_server_started(char* username, pid_t pid, int display, char* dbfield);
-char* raptor_sm_wm_started(char* username, pid_t pid, char* dbfield);
+int   raptor_sm_wm_started(char* username, pid_t pid, char* dbfield);
 int   raptor_sm_get_display_for_username(char* username);
 void  raptor_sm_wait_for_pid_exit(char* username, pid_t pid);
 char* raptor_sm_get_username_for_display_and_hostname(int display, char* hostname);
 void  raptor_sm_session_terminated(char* username);
-void  raptor_sm_wm_terminated(char* username);
+int   raptor_sm_wm_terminated(char* username);
 int   raptor_sm_get_new_unique_display(int mindisplay, int maxdisplay);
 bool  raptor_sm_sesslimit_reached(char* username);
 char  raptor_sm_set_session_state(int display, int state);
 void  raptor_sm_run_remote_desktop(char* username, int display, char* executable);
 void  raptor_sm_terminate_server(char* username);
 char* raptor_sm_get_hostname_for_display(int display);
-void  raptor_sm_stats_report_server_start(char* hostname);
+int   raptor_sm_stats_report_server_start(char* hostname);
-void  raptor_sm_stats_report_server_stop(char* hostname);
+int   raptor_sm_stats_report_server_stop(char* hostname);