Do not use atoi

includes/mem_scan.h

@@ -35,6 +35,6 @@
 
 #include <sqlite3.h>
 
-void scan_mem(sqlite3 *db, char* arg_pid);
+void scan_mem(sqlite3 *db, int pid);
 
 #endif //MEM_SCAN_H_

src/chericat.c

@@ -31,6 +31,7 @@
  */
 
 #include <ctype.h>
+#include <err.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <getopt.h>
@@ -54,25 +55,25 @@
 
 static void usage()
 {
-	fprintf(stderr, "Usage: chericat [-d <debug level>] [-f <database name>] [-p <pid>] [-v]\n\t[-c <binary name>]\n"
-			"     debug level - 0 = No output; 1 = INFO; 2 = VERBOSE; 3 = TROUBLESHOOT\n"
-			"     pid - pid of the target process for a snapshot of caps info\n"
+	fprintf(stderr, "Usage: chericat [-d] [-f <database name>] [-p <pid>] [-v]\n\t[-c <binary name>]\n"
+			"     pid           - pid of the target process for a snapshot of caps info\n"
 			"     database name - name of the database to store data captured by chericat\n"
+			"     binary name   - name of the binary file for which to show the\n"
+			"                     capabilities located, with corresponding symbols\n"
 			"Options:\n"
-			"     -d Determine the level of debugging messages to be printed. If omitted,\n"
-			"        the default is INFO level\n"
+			"     -d Enable debugging output. Repeated -d's (up to 3) increase verbosity.\n" 
 			"     -f Provide the database name to capture the data collected by chericat.\n"
 			"        If omitted, an in-memory db is used\n"
 			"     -p Scan the mapped memory and persist the caps data to a database\n"
 			"     -v Show virtual summary info of capabilities in the target process,\n"
 			"        arranged in mmap order\n"
 			"     -c Show capabilities with corresponding symbols located in the provided\n"
-			"        binary\n\n");
+			"        binary\n");
 }
 
 static struct option long_options[] = 
 {
-	{"debug_level", required_argument, 0, 'd'},
+	{"debug_level", no_argument, 0, 'd'},
 	{"database_name", required_argument, 0, 'f'},
 	{"scan_mem", required_argument, 0, 'p'},
 	{"caps_summary", no_argument, 0, 'v'},
@@ -88,7 +89,7 @@ main(int argc, char *argv[])
 	argc = xo_parse_args(argc, argv);
 
 	int optindex;
-	int opt = getopt_long(argc, argv, "d:f:p:vc:", long_options, &optindex);
+	int opt = getopt_long(argc, argv, "df:p:vc:", long_options, &optindex);
 
 	if (opt == -1) {
 		usage();
@@ -97,7 +98,8 @@ main(int argc, char *argv[])
 
 	sqlite3 *db = NULL;
 
-	set_print_level(INFO);
+	static int debug_level = 0;
+	set_print_level(debug_level);
 
 	while (opt != -1) {
 		switch (opt)
@@ -112,7 +114,15 @@ main(int argc, char *argv[])
                		 		return (1);
         			}
 			}
-			scan_mem(db, optarg);
+
+			char *pEnd;
+			long int pid = strtol(optarg, &pEnd, 10);
+
+			if (*pEnd != '\0') {
+				errx(1, "%s is not a valid pid", optarg);
+			}
+
+			scan_mem(db, pid);
 			break;
 		case 'v':
 			if (db == NULL) {
@@ -145,13 +155,14 @@ main(int argc, char *argv[])
 			strcpy(dbname, optarg);
 			break;
 		case 'd':
-			set_print_level(atoi(optarg));
+			debug_level++;
+			set_print_level(debug_level);
 			break;
 		default:
 			usage();
 			exit(0);
 		}
-		opt = getopt_long(argc, argv, "d:f:p:vc:", long_options, &optindex);
+		opt = getopt_long(argc, argv, "df:p:vc:", long_options, &optindex);
 	}
 
 	if (db != NULL) {

src/db_process.c

@@ -74,7 +74,7 @@ int db_table_exists(sqlite3 *db, char *tname)
 	rc = sqlite3_prepare_v2(db, check_table_q, -1, &stmt, NULL);
 
 	if (rc != SQLITE_OK) {
-		errx(1, "SQL error: %s\n", sqlite3_errmsg(db));
+		errx(1, "SQL error: %s", sqlite3_errmsg(db));
 	}
 
 	sqlite3_bind_text(stmt, 1, tname, -1, SQLITE_TRANSIENT);
@@ -88,7 +88,7 @@ int db_table_exists(sqlite3 *db, char *tname)
 		debug_print(VERBOSE, "%s does not exist in db %s\n", tname, dbname);
 	} else {
 		sqlite3_finalize(stmt);
-		errx(1, "SQL error: %s\n", sqlite3_errmsg(db));
+		errx(1, "SQL error: %s", sqlite3_errmsg(db));
 	}
 
 	sqlite3_finalize(stmt);
@@ -248,6 +248,23 @@ sym_info *all_sym_info;
 
 int all_vm_info_index, all_cap_info_index, all_sym_info_index;
 
+/*
+ * convert_str_to_int
+ * A convenient function to check if we can convert the 
+ * provided string to a long integer, and exit the program 
+ * early when an invalid value is detected.
+ */
+static int convert_str_to_int(char *str, char *err_msg)
+{
+	char *pEnd;
+	int int_val = strtol(str, &pEnd, 10);
+	if (*pEnd != '\0') {
+		fprintf(stderr, "DB processing - Expected an integer, got: %s\n", str);
+		errx(1, "%s", err_msg);
+	}
+	return int_val;
+}
+
 /*
  * vm_query_callback
  * SQLite callback routine to traverse results from sqlite_exec()
@@ -267,9 +284,16 @@ static int vm_info_query_callback(void *all_vm_info_ptr, int argc, char **argv,
         vm_info_captured.start_addr = strdup(argv[0]);
         vm_info_captured.end_addr = strdup(argv[1]);
         vm_info_captured.mmap_path = strdup(argv[2]);
-     	vm_info_captured.kve_protection = atoi(argv[3]);
-     	vm_info_captured.mmap_flags = atoi(argv[4]); 
-        vm_info_captured.vnode_type = atoi(argv[5]);
+
+	// Checking that the received values are valid integers
+	// as expected from the vm_info_captured data structure. 
+	// If the check fails, it means the data is invalid. Without
+	// a more sophisticated logic in place to handle invalid data, 
+	// we will just quit the program and let users to examine 
+	// and fix the data before running this function again.
+	vm_info_captured.kve_protection = convert_str_to_int(argv[3], "kve_protection is invalid");
+     	vm_info_captured.mmap_flags = convert_str_to_int(argv[4], "mmap_flags is invalid"); 
+        vm_info_captured.vnode_type = convert_str_to_int(argv[5], "vnode_type is invalid");
 
 	vm_info_captured.bss_addr = argv[6] == NULL ? NULL : strdup(argv[6]);
         vm_info_captured.bss_size = argv[7] == NULL ? NULL : strdup(argv[7]);
@@ -364,54 +388,39 @@ int vm_info_count(sqlite3 *db)
          * determine the size of the struct array for holding all the vm entries */
         char *count;
         sql_query_exec(db, "SELECT COUNT(*) FROM vm;", vm_info_count_query_callback, &count);
-
-	int result_count = atoi(count);
+	int result_count = convert_str_to_int(count, "Query to get count from vm returned an invalid value");
 	free(count);
 	return result_count;
 }
 
 int cap_info_count(sqlite3 *db)
 {
-	/*
-	if (0 == db_table_exists(db, "cap_info")) {
-		errx(1, "cap_info table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "cap_info");
 
         /* Obtain how many vm items from the database first, we can then use it to
          * determine the size of the struct array for holding all the vm entries */
         char *count;
 	sql_query_exec(db, "SELECT COUNT(*) FROM cap_info;", cap_info_count_query_callback, &count);
-
-        int result_count = atoi(count);
+	int result_count = convert_str_to_int(count, "Query to get count from cap_info returned an invalid value");
 	free(count);
 	return result_count;
 }
 
 int sym_info_count(sqlite3 *db)
 {
-	/*
-	if (0 == db_table_exists(db, "elf_sym")) {
-		errx(1, "elf_sym table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "elf_sym");
 
         /* Obtain how many vm items from the database first, we can then use it to
          * determine the size of the struct array for holding all the vm entries */
         char *count;
 	sql_query_exec(db, "SELECT COUNT(*) FROM elf_sym;", sym_info_count_query_callback, &count);
-
-        int result_count = atoi(count);
+	int result_count = convert_str_to_int(count, "Query to get count from elf_sym returned an invalid value");
 	free(count);
 	return result_count;
 }
 
 int cap_info_for_lib_count(sqlite3 *db, char *lib)
 {      
-        /*	
-	if (0 == db_table_exists(db, "cap_info")) {
-		errx(1, "cap_info table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "cap_info");
 
         /* Obtain how many vm items from the database first, we can then use it to
@@ -421,19 +430,14 @@ int cap_info_for_lib_count(sqlite3 *db, char *lib)
 	char *query;
 	asprintf(&query, "SELECT COUNT(*) FROM cap_info WHERE cap_loc_path LIKE \"%%%s%%\";", lib);
         sql_query_exec(db, query, cap_info_count_query_callback, &count);
-
-        int result_count = atoi(count);
+	int result_count = convert_str_to_int(count, "Query to get count from cap_info returned an invalid value");
         free(query);
 	free(count);
         return result_count;
 }
 
 int get_all_vm_info(sqlite3 *db, vm_info **all_vm_info_ptr)
 {
-	/*
-	if (0 == db_table_exists(db, "vm")) {
-		errx(1, "vm table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "vm");
 
 	int vm_count = vm_info_count(db);
@@ -451,10 +455,6 @@ int get_all_vm_info(sqlite3 *db, vm_info **all_vm_info_ptr)
 
 int get_all_cap_info(sqlite3 *db, cap_info **all_cap_info_ptr)
 {
-	/*
-	if (0 == db_table_exists(db, "cap_info")) {
-		errx(1, "cap_info table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "cap_info");
 
 	int cap_count = cap_info_count(db);
@@ -472,10 +472,6 @@ int get_all_cap_info(sqlite3 *db, cap_info **all_cap_info_ptr)
 
 int get_all_sym_info(sqlite3 *db, sym_info **all_sym_info_ptr)
 {
-	/*
-	if (0 == db_table_exists(db, "elf_sym")) {
-		errx(1, "elf_sym table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "elf_sym");
 
 	int sym_count = sym_info_count(db);
@@ -493,10 +489,6 @@ int get_all_sym_info(sqlite3 *db, sym_info **all_sym_info_ptr)
 
 int get_cap_info_for_lib(sqlite3 *db, cap_info **cap_info_captured_ptr, char *lib)
 {
-	/*
-	if (0 == db_table_exists(db, "cap_info")) {
-		errx(1, "cap_info table does not exist on db %s", dbname);
-	}*/
 	assert_db_table_exists(db, "cap_info");
 
 	int cap_count = cap_info_for_lib_count(db, lib);

src/mem_scan.c

@@ -65,10 +65,8 @@
  * When the -s option is used to attach this tool to a running process.
  * Uses ptrace to trace the mapped memory and persis the data to a db
  */
-void scan_mem(sqlite3 *db, char* arg_pid) 
+void scan_mem(sqlite3 *db, int pid) 
 {
-	int pid = atoi(arg_pid);
-
 	struct procstat *psp;
 	struct kinfo_proc *kipp;
 	struct kinfo_vmentry *freep, *kivp;
@@ -87,7 +85,7 @@ void scan_mem(sqlite3 *db, char* arg_pid)
 
 	freep = procstat_getvmmap(psp, kipp, &vmcnt);
 	if (freep == NULL) {
-		errx(1, "Unable to obtain the vm map information from process %d, does cherciat have the right privilege?", pid);
+		errx(1, "Unable to obtain the vm map information from process %d, does chericat have the right privilege?", pid);
 	}
 
 	create_vm_cap_db(db);

src/ptrace_utils.c

@@ -132,9 +132,12 @@ void read_data(int pid, void *addr, void *vptr, int len)
 	}
 }
 
-void read_lwps(char *arg_pid) {
-	int pid = atoi(arg_pid);
-
+/* 
+ * read_lwps
+ * Using ptrace to read info of kernel threads
+ */
+void read_lwps(int pid) 
+{
 	ptrace_attach(pid);
 
 	int nlwps;