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

BUG/MINOR: http-act: initialize http fmt head earlier

Browse Source 
In github issue #1850, Christian Ruppert reported a case of crash in
2.6 when failing to parse some http rules. This started to happen
with 2.6 commit dd7e6c6 ("BUG/MINOR: http-rules: completely free
incorrect TCP rules on error") but has some of its roots in 2.2
commit 2eb539687 ("MINOR: http-rules: Add release functions for
existing HTTP actions").

The cause is that when the release function is set for HTTP actions,
the rule->arg.http.fmt list head is not yet initialized, hence is
NULL, thus the release function crashes when it tries to iterate over
it. In fact this code was initially not written with the perspective
of releasing such elements upon error, so the arg list initialization
happened after error checking.

This patch just moves the list initialization just after setting the
release pointer and that's OK.

This patch must be backported to 2.6 since the problem is visible
there. It could be backported to 2.5 but the issue is not triggered
there without the first mentioned patch above that landed in 2.6, so
it will not bring any obvious benefit.

(cherry picked from commit 6a03a0d86d203342060f7072ae9271458ee0349c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
This commit is contained in:
Willy Tarreau 2022-09-02 19:19:01 +02:00
parent c79d0e6b74
commit 63e385ed8e
1 changed files with 7 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/http_act.c
View File

@ -179,6 +179,7 @@ static enum act_parse_ret parse_set_req_line(const char **args, int *orig_arg, s
}
rule->action_ptr = http_action_set_req_line;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
if (!*args[cur_arg] ||
(*args[cur_arg + 1] && strcmp(args[cur_arg + 1], "if") != 0 && strcmp(args[cur_arg + 1], "unless") != 0)) {
@ -186,7 +187,6 @@ static enum act_parse_ret parse_set_req_line(const char **args, int *orig_arg, s
return ACT_RET_PRS_ERR;
}
LIST_INIT(&rule->arg.http.fmt);
px->conf.args.ctx = ARGC_HRQ;
if (px->cap & PR_CAP_FE)
cap |= SMP_VAL_FE_HRQ_HDR;
@ -616,6 +616,7 @@ static enum act_parse_ret parse_replace_uri(const char **args, int *orig_arg, st
rule->action_ptr = http_action_replace_uri;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
if (!*args[cur_arg] || !*args[cur_arg+1] ||
(*args[cur_arg+2] && strcmp(args[cur_arg+2], "if") != 0 && strcmp(args[cur_arg+2], "unless") != 0)) {
@ -629,7 +630,6 @@ static enum act_parse_ret parse_replace_uri(const char **args, int *orig_arg, st
return ACT_RET_PRS_ERR;
}
LIST_INIT(&rule->arg.http.fmt);
px->conf.args.ctx = ARGC_HRQ;
if (px->cap & PR_CAP_FE)
cap |= SMP_VAL_FE_HRQ_HDR;
@ -680,6 +680,7 @@ static enum act_parse_ret parse_http_set_status(const char **args, int *orig_arg
rule->action = ACT_CUSTOM;
rule->action_ptr = action_http_set_status;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
/* Check if an argument is available */
if (!*args[*orig_arg]) {
@ -705,7 +706,6 @@ static enum act_parse_ret parse_http_set_status(const char **args, int *orig_arg
(*orig_arg)++;
}
LIST_INIT(&rule->arg.http.fmt);
return ACT_RET_PRS_OK;
}
@ -1318,6 +1318,7 @@ static enum act_parse_ret parse_http_auth(const char **args, int *orig_arg, stru
rule->flags |= ACT_FLAG_FINAL;
rule->action_ptr = http_action_auth;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
cur_arg = *orig_arg;
if (strcmp(args[cur_arg], "realm") == 0) {
@ -1330,7 +1331,6 @@ static enum act_parse_ret parse_http_auth(const char **args, int *orig_arg, stru
cur_arg++;
}
LIST_INIT(&rule->arg.http.fmt);
*orig_arg = cur_arg;
return ACT_RET_PRS_OK;
}
@ -1497,6 +1497,7 @@ static enum act_parse_ret parse_http_set_header(const char **args, int *orig_arg
rule->action_ptr = http_action_set_header;
}
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
cur_arg = *orig_arg;
if (!*args[cur_arg] || !*args[cur_arg+1]) {
@ -1506,7 +1507,6 @@ static enum act_parse_ret parse_http_set_header(const char **args, int *orig_arg
rule->arg.http.str = ist(strdup(args[cur_arg]));
LIST_INIT(&rule->arg.http.fmt);
if (rule->from == ACT_F_HTTP_REQ) {
px->conf.args.ctx = ARGC_HRQ;
@ -1606,6 +1606,7 @@ static enum act_parse_ret parse_http_replace_header(const char **args, int *orig
rule->action = 1; // replace-value
rule->action_ptr = http_action_replace_header;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
cur_arg = *orig_arg;
if (!*args[cur_arg] || !*args[cur_arg+1] || !*args[cur_arg+2]) {
@ -1614,7 +1615,6 @@ static enum act_parse_ret parse_http_replace_header(const char **args, int *orig
}
rule->arg.http.str = ist(strdup(args[cur_arg]));
LIST_INIT(&rule->arg.http.fmt);
cur_arg++;
if (!(rule->arg.http.re = regex_comp(args[cur_arg], 1, 1, err))) {
@ -1709,6 +1709,7 @@ static enum act_parse_ret parse_http_del_header(const char **args, int *orig_arg
rule->action = PAT_MATCH_STR;
rule->action_ptr = http_action_del_header;
rule->release_ptr = release_http_action;
LIST_INIT(&rule->arg.http.fmt);
cur_arg = *orig_arg;
if (!*args[cur_arg]) {
@ -1719,7 +1720,6 @@ static enum act_parse_ret parse_http_del_header(const char **args, int *orig_arg
rule->arg.http.str = ist(strdup(args[cur_arg]));
px->conf.args.ctx = (rule->from == ACT_F_HTTP_REQ ? ARGC_HRQ : ARGC_HRS);
LIST_INIT(&rule->arg.http.fmt);
if (strcmp(args[cur_arg+1], "-m") == 0) {
cur_arg++;
if (!*args[cur_arg+1]) {