BUG/MEDIUM: cfgcond: limit recursion level in the condition expression parser

Oss-fuzz reports in issue 36328 that we can recurse too far by passing
extremely deep expressions to the ".if" parser. I thought we were still
limited to the 1024 chars per line, that would be highly sufficient, but
we don't have any limit now :-/

Let's just pass a maximum recursion counter to the recursive parsers.
It's decremented for each call and the expression fails if it reaches
zero. On the most complex paths it can add 3 levels per parenthesis,
so with a limit of 1024, that's roughly 343 nested sub-expressions that
are supported in the worst case. That's more than sufficient, for just
a few kB of RAM.

No backport is needed.
This commit is contained in:
Willy Tarreau 2021-07-20 17:58:34 +02:00
parent 048368ef6f
commit dc70c18ddc
3 changed files with 46 additions and 15 deletions

View File

@ -26,15 +26,15 @@
#include <haproxy/cfgcond-t.h>
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_parse_cond_term(const char **text, struct cfg_cond_term **term, char **err, const char **errptr, int maxdepth);
int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err);
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_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err, const char **errptr, int maxdepth);
int cfg_eval_cond_and(struct cfg_cond_and *expr, char **err);
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_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **err, const char **errptr, int maxdepth);
int cfg_eval_cond_expr(struct cfg_cond_expr *expr, char **err);
void cfg_free_cond_expr(struct cfg_cond_expr *expr);

View File

@ -106,6 +106,11 @@
// This should cover at least 5 + twice the # of data_types
#define MAX_CLI_ARGS 64
// max recursion levels in config condition evaluations
// (note that binary operators add one recursion level, and
// that parenthesis may add two).
#define MAX_CFG_RECURSION 1024
// max # of matches per regexp
#define MAX_MATCH 10

View File

@ -68,9 +68,11 @@ void cfg_free_cond_term(struct cfg_cond_term *term)
* untouched on failure. On success, the caller must free <term> using
* cfg_free_cond_term(). An error will be set in <err> on error, and only
* in this case. In this case the first bad character will be reported in
* <errptr>.
* <errptr>. <maxdepth> corresponds to the maximum recursion depth permitted,
* it is decremented on each recursive call and the parsing will fail one
* reaching <= 0.
*/
int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **err, const char **errptr)
int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **err, const char **errptr, int maxdepth)
{
struct cfg_cond_term *t;
const char *in = *text;
@ -86,6 +88,10 @@ int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **e
if (!*in) /* empty term does not parse */
return 0;
*term = NULL;
if (maxdepth <= 0)
goto fail0;
t = *term = calloc(1, sizeof(**term));
if (!t) {
memprintf(err, "memory allocation error while parsing conditional expression '%s'", *text);
@ -117,7 +123,7 @@ int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **e
t->args = NULL;
do { in++; } while (*in == ' ' || *in == '\t');
ret = cfg_parse_cond_expr(&in, &t->expr, err, errptr);
ret = cfg_parse_cond_expr(&in, &t->expr, err, errptr, maxdepth - 1);
if (ret == -1)
goto fail2;
if (ret == 0)
@ -275,9 +281,11 @@ void cfg_free_cond_expr(struct cfg_cond_expr *expr)
* on failure. On success, the caller will have to free all lower-level
* allocated structs using cfg_free_cond_expr(). An error will be set in
* <err> on error, and only in this case. In this case the first bad
* character will be reported in <errptr>.
* character will be reported in <errptr>. <maxdepth> corresponds to the
* maximum recursion depth permitted, it is decremented on each recursive
* call and the parsing will fail one reaching <= 0.
*/
int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err, const char **errptr)
int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err, const char **errptr, int maxdepth)
{
struct cfg_cond_and *e;
const char *in = *text;
@ -286,13 +294,21 @@ int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err
if (!*in) /* empty expr does not parse */
return 0;
*expr = NULL;
if (maxdepth <= 0) {
memprintf(err, "unparsable conditional sub-expression '%s'", in);
if (errptr)
*errptr = in;
goto done;
}
e = *expr = calloc(1, sizeof(**expr));
if (!e) {
memprintf(err, "memory allocation error while parsing conditional expression '%s'", *text);
goto done;
}
ret = cfg_parse_cond_term(&in, &e->left, err, errptr);
ret = cfg_parse_cond_term(&in, &e->left, err, errptr, maxdepth - 1);
if (ret == -1) // parse error, error already reported
goto done;
@ -320,7 +336,7 @@ int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err
while (*in == ' ' || *in == '\t')
in++;
ret = cfg_parse_cond_and(&in, &e->right, err, errptr);
ret = cfg_parse_cond_and(&in, &e->right, err, errptr, maxdepth - 1);
if (ret > 0)
*text = in;
done:
@ -338,9 +354,11 @@ int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err
* on failure. On success, the caller will have to free all lower-level
* allocated structs using cfg_free_cond_expr(). An error will be set in
* <err> on error, and only in this case. In this case the first bad
* character will be reported in <errptr>.
* character will be reported in <errptr>. <maxdepth> corresponds to the
* maximum recursion depth permitted, it is decremented on each recursive call
* and the parsing will fail one reaching <= 0.
*/
int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **err, const char **errptr)
int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **err, const char **errptr, int maxdepth)
{
struct cfg_cond_expr *e;
const char *in = *text;
@ -349,13 +367,21 @@ int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **e
if (!*in) /* empty expr does not parse */
return 0;
*expr = NULL;
if (maxdepth <= 0) {
memprintf(err, "unparsable conditional expression '%s'", in);
if (errptr)
*errptr = in;
goto done;
}
e = *expr = calloc(1, sizeof(**expr));
if (!e) {
memprintf(err, "memory allocation error while parsing conditional expression '%s'", *text);
goto done;
}
ret = cfg_parse_cond_and(&in, &e->left, err, errptr);
ret = cfg_parse_cond_and(&in, &e->left, err, errptr, maxdepth - 1);
if (ret == -1) // parse error, error already reported
goto done;
@ -383,7 +409,7 @@ int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **e
while (*in == ' ' || *in == '\t')
in++;
ret = cfg_parse_cond_expr(&in, &e->right, err, errptr);
ret = cfg_parse_cond_expr(&in, &e->right, err, errptr, maxdepth - 1);
if (ret > 0)
*text = in;
done:
@ -440,7 +466,7 @@ int cfg_eval_condition(char **args, char **err, const char **errptr)
if (!*text) /* note: empty = false */
return 0;
ret = cfg_parse_cond_expr(&text, &expr, err, errptr);
ret = cfg_parse_cond_expr(&text, &expr, err, errptr, MAX_CFG_RECURSION);
if (ret != 0) {
if (ret == -1) // parse error, error already reported
goto done;