From 693a8f7ea2b008a43ff28751a9a25310171fe10e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 20 Aug 2024 19:52:11 +0200 Subject: [PATCH] Avoid (unlikely) memory leaking and ensure we properly reset the config type to a non-allocated type after free'ing. Signed-off-by: DL6ER --- src/api/config.c | 1 + src/config/cli.c | 2 +- src/config/config.c | 13 ++++++++++--- src/config/config.h | 2 -- src/config/legacy_reader.c | 8 ++++++++ src/config/password.c | 21 +++++++++++++++++---- src/config/setupVars.c | 2 +- 7 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/api/config.c b/src/api/config.c index c5d005bb2a..5b12dd10f4 100644 --- a/src/api/config.c +++ b/src/api/config.c @@ -279,6 +279,7 @@ static const char *getJSONvalue(struct conf_item *conf_item, cJSON *elem, struct free(conf_item->v.s); // Set item conf_item->v.s = strdup(elem->valuestring); + conf_item->t = CONF_STRING_ALLOCATED; // allocated now log_debug(DEBUG_CONFIG, "%s = \"%s\"", conf_item->k, conf_item->v.s); break; } diff --git a/src/config/cli.c b/src/config/cli.c index 0bcc249dee..7c2de6bd09 100644 --- a/src/config/cli.c +++ b/src/config/cli.c @@ -182,7 +182,7 @@ static bool readStringValue(struct conf_item *conf_item, const char *value, stru // Free old password hash if it was allocated if(conf_item->t == CONF_STRING_ALLOCATED) - free(conf_item->v.s); + free(conf_item->v.s); // Store new password hash conf_item->v.s = pwhash; diff --git a/src/config/config.c b/src/config/config.c index 42d1f7c784..d3d28c80b3 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -42,6 +42,8 @@ uint8_t last_checksum[SHA256_DIGEST_SIZE] = { 0 }; // Private prototypes static bool port_in_use(const in_port_t port); +static void reset_config_default(struct conf_item *conf_item); +static void initConfig(struct config *conf); // Set debug flags from config struct to global debug_flags array // This is called whenever the config is reloaded and debug flags may have @@ -158,7 +160,10 @@ void free_config_path(char **paths) for(unsigned int i = 0; i < MAX_CONFIG_PATH_DEPTH; i++) if(paths[i] != NULL) + { free(paths[i]); + paths[i] = NULL; + } } bool __attribute__ ((pure)) check_paths_equal(char **paths1, char **paths2, unsigned int max_level) @@ -369,6 +374,8 @@ void free_config(struct config *conf) break; case CONF_STRING_ALLOCATED: free(copy_item->v.s); + copy_item->v.s = NULL; + copy_item->t = CONF_STRING; // not allocated anymore break; case CONF_JSON_STRING_ARRAY: cJSON_Delete(copy_item->v.json); @@ -377,7 +384,7 @@ void free_config(struct config *conf) } } -void initConfig(struct config *conf) +static void initConfig(struct config *conf) { if(config_initialized) return; @@ -1492,7 +1499,7 @@ void initConfig(struct config *conf) // Initialize config value with default one for all *except* the log file path if(conf_item != &conf->files.log.ftl) - reset_config(conf_item); + reset_config_default(conf_item); // Parse and split paths conf_item->p = gen_config_path(conf_item->k, '.'); @@ -1541,7 +1548,7 @@ void initConfig(struct config *conf) } } -void reset_config(struct conf_item *conf_item) +static void reset_config_default(struct conf_item *conf_item) { if(conf_item->t == CONF_JSON_STRING_ARRAY) { diff --git a/src/config/config.h b/src/config/config.h index 0839c1b058..811f48b393 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -348,8 +348,6 @@ extern struct config config; // Defined in config.c void set_debug_flags(struct config *conf); void set_all_debug(struct config *conf, const bool status); -void initConfig(struct config *conf); -void reset_config(struct conf_item *conf_item); bool readFTLconf(struct config *conf, const bool rewrite); bool getLogFilePath(void); struct conf_item *get_conf_item(struct config *conf, const unsigned int n); diff --git a/src/config/legacy_reader.c b/src/config/legacy_reader.c index 8e64cfedc3..4a5b16050b 100644 --- a/src/config/legacy_reader.c +++ b/src/config/legacy_reader.c @@ -66,6 +66,10 @@ bool getLogFilePathLegacy(struct config *conf, FILE *fp) // No option set => use default log location if(buffer == NULL) { + // Free previously allocated memory (if any) + if(conf->files.log.ftl.t == CONF_STRING_ALLOCATED) + free(conf->files.log.ftl.v.s); + // Use standard path if no custom path was obtained from the config file conf->files.log.ftl.v.s = strdup("/var/log/pihole/FTL.log"); conf->files.log.ftl.t = CONF_STRING_ALLOCATED; @@ -92,6 +96,10 @@ bool getLogFilePathLegacy(struct config *conf, FILE *fp) conf->files.log.ftl.v.s = NULL; conf->files.log.ftl.t = CONF_STRING; log_info("Using syslog facility"); + + // Free buffer + if(val_buffer != NULL) + free(val_buffer); } // Set string if memory allocation was successful and a value was read diff --git a/src/config/password.c b/src/config/password.c index f90bc4ade3..526c1229be 100644 --- a/src/config/password.c +++ b/src/config/password.c @@ -199,6 +199,14 @@ static char * __attribute__((malloc)) balloon_password(const char *password, // Build PHC string-like output (output string is 101 bytes long (measured)) char *output = calloc(128, sizeof(char)); + + if(output == NULL || salt_base64 == NULL || scratch_base64 == NULL) + { + log_err("Error while allocating memory for PHC string: %s", strerror(errno)); + goto clean_and_exit; + } + + // Generate PHC string int size = snprintf(output, 128, "$BALLOON-SHA256$v=1$s=%zu,t=%zu$%s$%s", s_cost, t_cost, @@ -213,11 +221,15 @@ static char * __attribute__((malloc)) balloon_password(const char *password, } clean_and_exit: - free(scratch); - free(salt_base64); - free(scratch_base64); + // Clean up + if(scratch != NULL) + free(scratch); + if(salt_base64 != NULL) + free(salt_base64); + if(scratch_base64 != NULL) + free(scratch_base64); - return output; + return output; // may be NULL on failure (unlikely) } // Parse a PHC string and return the parameters and hash @@ -653,6 +665,7 @@ bool set_and_check_password(struct conf_item *conf_item, const char *password) // Set item conf_item->v.s = pwhash; + conf_item->t = CONF_STRING_ALLOCATED; log_debug(DEBUG_CONFIG, "Set %s to \"%s\"", conf_item->k, conf_item->v.s); return true; diff --git a/src/config/setupVars.c b/src/config/setupVars.c index e0c5400992..18fedab105 100644 --- a/src/config/setupVars.c +++ b/src/config/setupVars.c @@ -553,7 +553,7 @@ void importsetupVarsConf(void) get_conf_string_from_setupVars("DHCP_LEASETIME", &config.dhcp.leaseTime); // If the DHCP lease time is set to "24", it is interpreted as "24h". - // This is some relic from the past that may still be present in some + // This is some relict from the past that may still be present in some // setups if(strcmp(config.dhcp.leaseTime.v.s, "24") == 0) {