slipenkomk/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: cfgcond: revisit the condition freeing mechanism to avoid a leak

Browse Source 
The cfg_free_cond_{term,and,expr}() functions used to take a pointer to
the pointer to be freed in order to replace it with a NULL once done.
But this doesn't cope well with freeing lists as it would require
recursion which the current code tried to avoid.

Let's just change the API to free the area and let the caller set the NULL.

This leak was reported by oss-fuzz (issue 36265).
This commit is contained in:
Willy Tarreau 2021-07-17 18:46:30 +02:00
parent 69a23ae091
commit f1db20c473
2 changed files with 39 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/haproxy/cfgcond.h
View File

@ -28,15 +28,15 @@
const struct cond_pred_kw *cfg_lookup_cond_pred(const char *str);
int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **err, const char **errptr);
int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err);
void cfg_free_cond_term(struct cfg_cond_term **term);
void cfg_free_cond_term(struct cfg_cond_term *term);
int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err, const char **errptr);
int cfg_eval_cond_and(struct cfg_cond_and *expr, char **err);
void cfg_free_cond_and(struct cfg_cond_and **expr);
void cfg_free_cond_and(struct cfg_cond_and *expr);
int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **err, const char **errptr);
int cfg_eval_cond_expr(struct cfg_cond_expr *expr, char **err);
void cfg_free_cond_expr(struct cfg_cond_expr **expr);
void cfg_free_cond_expr(struct cfg_cond_expr *expr);
int cfg_eval_condition(char **args, char **err, const char **errptr);

57
src/cfgcond.c
View File

@ -46,17 +46,19 @@ const struct cond_pred_kw *cfg_lookup_cond_pred(const char *str)
}
/* Frees <term> and its args. NULL is supported and does nothing. */
void cfg_free_cond_term(struct cfg_cond_term **term)
void cfg_free_cond_term(struct cfg_cond_term *term)
{
if (!term || !*term)
if (!term)
return;
if ((*term)->type == CCTT_PAREN)
cfg_free_cond_expr(&(*term)->expr);
if (term->type == CCTT_PAREN) {
cfg_free_cond_expr(term->expr);
term->expr = NULL;
}
free_args((*term)->args);
free((*term)->args);
ha_free(term);
free_args(term->args);
free(term->args);
free(term);
}
/* Parse an indirect input text as a possible config condition term.
@ -158,7 +160,8 @@ int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **e
if (errptr)
*errptr = *text;
fail2:
cfg_free_cond_term(term);
cfg_free_cond_term(*term);
*term = NULL;
return -1;
}
@ -240,20 +243,28 @@ int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err)
/* Frees <expr> and its terms and args. NULL is supported and does nothing. */
void cfg_free_cond_and(struct cfg_cond_and **expr)
void cfg_free_cond_and(struct cfg_cond_and *expr)
{
while (expr && *expr) {
cfg_free_cond_term(&(*expr)->left);
expr = &(*expr)->right;
struct cfg_cond_and *prev;
while (expr) {
cfg_free_cond_term(expr->left);
prev = expr;
expr = expr->right;
free(prev);
}
}
/* Frees <expr> and its terms and args. NULL is supported and does nothing. */
void cfg_free_cond_expr(struct cfg_cond_expr **expr)
void cfg_free_cond_expr(struct cfg_cond_expr *expr)
{
while (expr && *expr) {
cfg_free_cond_and(&(*expr)->left);
expr = &(*expr)->right;
struct cfg_cond_expr *prev;
while (expr) {
cfg_free_cond_and(expr->left);
prev = expr;
expr = expr->right;
free(prev);
}
}
@ -313,8 +324,10 @@ int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err
if (ret > 0)
*text = in;
done:
if (ret < 0)
cfg_free_cond_and(expr);
if (ret < 0) {
cfg_free_cond_and(*expr);
*expr = NULL;
}
return ret;
}
@ -374,8 +387,10 @@ int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **e
if (ret > 0)
*text = in;
done:
if (ret < 0)
cfg_free_cond_expr(expr);
if (ret < 0) {
cfg_free_cond_expr(*expr);
*expr = NULL;
}
return ret;
}
@ -450,6 +465,6 @@ int cfg_eval_condition(char **args, char **err, const char **errptr)
if (errptr)
*errptr = text;
done:
cfg_free_cond_expr(&expr);
cfg_free_cond_expr(expr);
return ret;
}