From a712e550c5ba1409eed440e5b49b642fcb4dbbdb Mon Sep 17 00:00:00 2001 From: William Brawner Date: Tue, 31 Dec 2019 16:33:49 -0600 Subject: [PATCH] Fix various issues related to memory misuse --- PiHelper/cli.c | 45 ++++++++++++++++++++++++++------------------- PiHelper/cli.h | 2 +- PiHelper/config.c | 37 +++++++++++++++++++++++++++++-------- PiHelper/config.h | 3 ++- PiHelper/log.c | 3 ++- PiHelper/network.c | 5 ++++- PiHelper/pihelper.c | 5 +++++ PiHelper/pihelper.h | 2 ++ 8 files changed, 71 insertions(+), 31 deletions(-) diff --git a/PiHelper/cli.c b/PiHelper/cli.c index c9898f7..215d6b5 100644 --- a/PiHelper/cli.c +++ b/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() { diff --git a/PiHelper/cli.h b/PiHelper/cli.h index 18a5afc..69189c2 100644 --- a/PiHelper/cli.h +++ b/PiHelper/cli.h @@ -18,7 +18,7 @@ */ #include -static char * shortopts = "cd:ef:hqv"; +static char * shortopts = "cd::ef:hqv"; static struct option longopts[] = { { "configure", no_argument, NULL, 'c' }, diff --git a/PiHelper/config.c b/PiHelper/config.c index 793c188..e56db5c 100644 --- a/PiHelper/config.c +++ b/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); +} + diff --git a/PiHelper/config.h b/PiHelper/config.h index 3aa9571..a6f2b45 100644 --- a/PiHelper/config.h +++ b/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 diff --git a/PiHelper/log.c b/PiHelper/log.c index 69e7ba9..b9e74ea 100644 --- a/PiHelper/log.c +++ b/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); } diff --git a/PiHelper/network.c b/PiHelper/network.c index c22a89f..03ac95c 100644 --- a/PiHelper/network.c +++ b/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) { diff --git a/PiHelper/pihelper.c b/PiHelper/pihelper.c index 3464778..80d81ba 100644 --- a/PiHelper/pihelper.c +++ b/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); +} + diff --git a/PiHelper/pihelper.h b/PiHelper/pihelper.h index c91a958..5c680b0 100644 --- a/PiHelper/pihelper.h +++ b/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