Fix various issues related to memory misuse

PiHelper/cli.c

@@ -26,9 +26,9 @@
 
 int main(int argc, char ** argv) {
 
-    bool configure, enable;
+    bool configure = false, enable = false;
     char * disable = NULL;
-    char * config_path;
+    char * config_path = NULL;
     char ch;
     while ((ch = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) {
         switch(ch) {
@@ -41,7 +41,8 @@ int main(int argc, char ** argv) {
                     strncpy(disable, optarg, strlen(optarg));
                     disable[strlen(optarg)] = '\0';
                 } else {
-                    disable = "";
+                    disable = malloc(1);
+                    disable[0] = '\0';
                 }
                 break;
             case 'e':
@@ -57,7 +58,8 @@ int main(int argc, char ** argv) {
                     strncpy(config_path, cwd, strlen(cwd));
                     config_path[strlen(cwd)] = '/';
                     strncpy(&(config_path[strlen(cwd) + 1]), optarg, strlen(optarg));
-                    config_path[full_path_len] = '\0';
+                    config_path[full_path_len - 1] = '\0';
+                    free(cwd);
                 } else {
                     // This is an absolute path, copy as-is
                     config_path = malloc(strlen(optarg) + 1);
@@ -82,10 +84,9 @@ int main(int argc, char ** argv) {
         int path_len = strlen(home_dir) + strlen(DEFAULT_CONFIG_PATH);
         config_path = malloc(path_len + 1);
         sprintf(config_path, "%s%s", home_dir, DEFAULT_CONFIG_PATH);
-        config_path[path_len + 1] = '\0';
+        config_path[path_len] = '\0';
     }
-    FILE * config_file = fopen(config_path, "r+");
-    if (config_file == NULL) {
+    if (access(config_path, F_OK)) {
         char * user_input = malloc(2);
         // Intentionally using printf here to ensure that this is always printed
         printf("No Pi-Helper configuration found. Would you like to create it now? [Y/n] ");
@@ -95,7 +96,10 @@ int main(int argc, char ** argv) {
                 || strstr(user_input, "y") == user_input
            ) {
             configure = true;
+            free(user_input);
         } else {
+            free(config_path);
+            free(user_input);
             return 1;
         }
     } else {
@@ -108,22 +112,25 @@ int main(int argc, char ** argv) {
     } else {
         config = read_config(config_path);
     }
-
+    int retval;
     if (config == NULL) {
         write_log(PIHELPER_LOG_ERROR, "Failed to parse Pi-Helper config at %s", config_path);
-        exit(1);
+        retval = 1;
+    } else if (enable && disable != NULL) {
+        print_usage();
+        retval =  PIHELPER_INVALID_COMMANDS;
+    } else if (enable) {
+        retval = enable_pihole(config);
+    } else if (disable != NULL) {
+        retval =  disable_pihole(config, disable);
+        free(disable);
+    } else {
+        retval = get_status(config);
     }
 
-    if (enable && disable != NULL) {
-        print_usage();
-        return PIHELPER_INVALID_COMMANDS;
-    } else if (enable) {
-        return enable_pihole(config);
-    } else if (disable != NULL) {
-        return disable_pihole(config, disable);
-    } else {
-        return get_status(config);
-    }
+    free(config_path);
+    pihelper_free_config(config);
+    return retval;
 }
 
 void print_usage() {

PiHelper/cli.h

@@ -18,7 +18,7 @@
  */
 #include <getopt.h>
 
-static char * shortopts = "cd:ef:hqv";
+static char * shortopts = "cd::ef:hqv";
 
 static struct option longopts[] = {
         { "configure",  no_argument,            NULL,           'c' },

PiHelper/config.c

@@ -83,20 +83,29 @@ pihole_config * read_config(char * config_path) {
     if (strstr(host, "host=") == NULL || strlen(host) < 7) {
         write_log(PIHELPER_LOG_DEBUG, "Config file contains invalid host: %s", host);
         write_log(PIHELPER_LOG_ERROR, "Invalid config file");
-        exit(1);
+        free_config(config);
+        fclose(config_file);
+        return NULL;
     }
-    config->host = calloc(1, strlen(host) - 7);
+    config->host = calloc(1, strlen(host) - 5);
     strncpy(config->host, host + 5, strlen(host) - 6);
-    char api_key[74];
+    config->host[strlen(host) - 6] = '\0';
+    char * api_key = calloc(1, 74);
     fgets(api_key, 74, config_file);
-    if (strstr(api_key, "api-key=") == NULL) {
+    if (strstr(api_key, "api-key=") == NULL
+            || strlen(api_key) < 9) {
         write_log(PIHELPER_LOG_DEBUG, "Config file contains invalid api key: %s", api_key);
-        write_log(PIHELPER_LOG_ERROR, "Invalid config file");
-        exit(1);
+        write_log(PIHELPER_LOG_WARN, "The config file is missing a valid API key. Authenticated operations won't work.");
+        free(api_key);
+        fclose(config_file);
+        return config;
     }
     config->api_key = calloc(1, strlen(api_key) - 8);
     strncpy(config->api_key, api_key + 8, strlen(api_key) - 9);
+    config->api_key[strlen(api_key) - 9] = '\0';
+    free(api_key);
     fclose(config_file);
+    write_log(PIHELPER_LOG_DEBUG, "Using host %s and api key %s", config->host, config->api_key);
     return config;
 }
 
@@ -107,7 +116,7 @@ pihole_config * configure_pihole(char * config_path) {
     }
     pihole_config * config = malloc(sizeof(pihole_config));
     config->host = calloc(1, _POSIX_HOST_NAME_MAX);
-    config->host[_POSIX_HOST_NAME_MAX] = '\0';
+    config->host[_POSIX_HOST_NAME_MAX - 1] = '\0';
     // Intentionally using printf to ensure this is always printed
     printf("Enter the hostname or ip address for your pi-hole: ");
     fgets(config->host, _POSIX_HOST_NAME_MAX, stdin);
@@ -116,12 +125,13 @@ pihole_config * configure_pihole(char * config_path) {
         config->host[strlen(config->host) - strlen(newline)] = '\0';
     }
     config->api_key = getpass("Enter the api key or web password for your pi-hole: ");
-    if (strlen(config->api_key) < 64) {
+    if (strlen(config->api_key) != 64) {
         // This is definitely not an API key, so hash it
         // The Pi-hole hashes twice so we do the same here
         char * first = hash_string(config->api_key);
         char * hash = hash_string(first);
         free(first);
+        free(config->api_key);
         config->api_key = hash;
     }
     // TODO: Make an authenticated request to verify that the credentials are valid and save the config
@@ -129,3 +139,14 @@ pihole_config * configure_pihole(char * config_path) {
     return config;
 }
 
+void free_config(pihole_config * config) {
+    if (config == NULL) return;
+    if (config->host != NULL) {
+        free(config->host);
+    }
+    if (config->api_key != NULL) {
+        free(config->api_key);
+    }
+    free(config);
+}
+

PiHelper/config.h

@@ -28,11 +28,12 @@ typedef struct {
     char * api_key;
 } pihole_config;
 
-
 void save_config(pihole_config * config, char * config_path);
 
 pihole_config * read_config(char * config_path);
 
 pihole_config * configure_pihole(char * config_path);
+
+void free_config(pihole_config * config);
 #endif
 

PiHelper/log.c

@@ -34,12 +34,13 @@ void write_log(int level, char * format, ...) {
     va_list args;
     va_start(args, format);
     int format_len = strlen(format);
-    char * new_format = malloc(format_len + 1);
+    char * new_format = malloc(format_len + 2);
     memcpy(new_format, format, format_len);
     new_format[format_len] = '\n';
     new_format[format_len + 1] = '\0';
     FILE *stream = level < PIHELPER_LOG_INFO ? stderr : stdout;
     vfprintf(stream, new_format, args);
     va_end(args);
+    free(new_format);
 }
 

PiHelper/network.c

@@ -38,6 +38,7 @@ int get_status(pihole_config * config) {
     write_log(PIHELPER_LOG_DEBUG, "Getting Pi-hole status…");
     char * formatted_host = prepend_scheme(config->host);
     char * response = get(formatted_host);
+    free(formatted_host);
     if (response == NULL) {
         write_log(PIHELPER_LOG_ERROR, "Failed to retrieve status for Pi-hole at %s\n", config->host);
         return 1;
@@ -54,6 +55,7 @@ int enable_pihole(pihole_config * config) {
     append_query_parameter(&formatted_host, AUTH_QUERY, config->api_key);
     append_query_parameter(&formatted_host, ENABLE_QUERY, NULL);
     char * response = get(formatted_host);
+    free(formatted_host);
     if (response == NULL) {
         return 1;
     } else {
@@ -73,6 +75,7 @@ int disable_pihole(pihole_config * config, char * duration) {
     append_query_parameter(&formatted_host, AUTH_QUERY, config->api_key);
     append_query_parameter(&formatted_host, DISABLE_QUERY, duration);
     char * response = get(formatted_host);
+    free(formatted_host);
     if (response == NULL) {
         return 1;
     } else {
@@ -150,7 +153,7 @@ static void parse_status(char * raw_json) {
         write_log(PIHELPER_LOG_ERROR, "Failed to parse JSON: %s", json_tokener_error_desc(jerr));
         return;
     }
-    json_object *status = json_object_new_object();
+    json_object *status;
     const char * status_string;
     if (json_pointer_get(jobj, "/status", &status) == 0
             && (status_string = json_object_get_string(status)) != NULL) {

PiHelper/pihelper.c

@@ -33,3 +33,8 @@ int pihelper_disable_pihole(pihole_config * config, char * duration) {
 void pihelper_set_log_level(int level) {
     set_log_level(level);
 }
+
+void pihelper_free_config(pihole_config * config) {
+    free_config(config);
+}
+

PiHelper/pihelper.h

@@ -32,5 +32,7 @@ int pihelper_get_status(pihole_config * config);
 int pihelper_enable_pihole(pihole_config * config);
 
 int pihelper_disable_pihole(pihole_config * config, char * duration);
+
+void pihelper_free_config(pihole_config * config);
 #endif