Skip to content

Commit

Permalink
Avoid (unlikely) memory leaking and ensure we properly reset the conf…
Browse files Browse the repository at this point in the history
…ig type to a non-allocated type after free'ing.

Signed-off-by: DL6ER <[email protected]>
  • Loading branch information
DL6ER committed Aug 20, 2024
1 parent 0c36f47 commit 693a8f7
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/api/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/config/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions src/config/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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, '.');
Expand Down Expand Up @@ -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)
{
Expand Down
2 changes: 0 additions & 2 deletions src/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions src/config/legacy_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
21 changes: 17 additions & 4 deletions src/config/password.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/config/setupVars.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down

0 comments on commit 693a8f7

Please sign in to comment.