BUG/MAJOR: checks: always check for end of list before proceeding
This is the most important fix of this series. There's a risk of endless
loop and crashes caused by the fact that we go past the head of the list
when skipping to next rule, without checking if it's still a valid element.
Most of the time, the ->action field is checked, which points to the proxy's
check_req pointer (generally NULL), meaning the element is confused with a
TCPCHK_ACT_SEND action.
The situation was accidently made worse with the addition of tcp-check
comment since it also skips list elements. However, since the action that
makes it go forward is TCPCHK_ACT_COMMENT (3), there's little chance to
see this as a valid pointer, except on 64-bit machines where it can match
the end of a check_req string pointer.
This fix heavily depends on previous cleanup and both must be backported
to 1.5 where the bug is present.
diff --git a/src/cfgparse.c b/src/cfgparse.c
index c18628a..7752fc7 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -4712,14 +4712,13 @@
l = (struct list *)&curproxy->tcpcheck_rules;
if (l->p != l->n) {
tcpcheck = (struct tcpcheck_rule *)l->n;
- while (tcpcheck->action == TCPCHK_ACT_COMMENT) {
+ while (&tcpcheck->list != &curproxy->tcpcheck_rules &&
+ tcpcheck->action == TCPCHK_ACT_COMMENT) {
tcpcheck = (struct tcpcheck_rule *)tcpcheck->list.n;
}
- /* we've reached the end of the list, and the list is full of comments */
- if (tcpcheck == (struct tcpcheck_rule *)l)
- tcpcheck = NULL;
- if (tcpcheck && tcpcheck->action != TCPCHK_ACT_CONNECT) {
+ if (&tcpcheck->list != &curproxy->tcpcheck_rules
+ && tcpcheck->action != TCPCHK_ACT_CONNECT) {
Alert("parsing [%s:%d] : first step MUST also be a 'connect' when there is a 'connect' step in the tcp-check ruleset.\n",
file, linenum);
err_code |= ERR_ALERT | ERR_FATAL;
diff --git a/src/checks.c b/src/checks.c
index cc29ac8..78f022f 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -2522,7 +2522,7 @@
next = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
- while (next->action == TCPCHK_ACT_COMMENT)
+ while (&next->list != head && next->action == TCPCHK_ACT_COMMENT)
next = (struct tcpcheck_rule *)next->list.n;
/* NULL if we're on the last rule */
@@ -2636,9 +2636,13 @@
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
- while (check->current_step->action == TCPCHK_ACT_COMMENT)
+ while (&check->current_step->list != head &&
+ check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+ if (&check->current_step->list == head)
+ break;
+
/* don't do anything until the connection is established */
if (!(conn->flags & CO_FL_CONNECTED)) {
/* update expire time, should be done by process_chk */
@@ -2694,8 +2698,12 @@
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
- while (check->current_step->action == TCPCHK_ACT_COMMENT)
+ while (&check->current_step->list != head &&
+ check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+
+ if (&check->current_step->list == head)
+ break;
} /* end 'send' */
else if (check->current_step->action == TCPCHK_ACT_EXPECT) {
if (unlikely(check->result == CHK_RES_FAILED))
@@ -2789,9 +2797,13 @@
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
- while (check->current_step->action == TCPCHK_ACT_COMMENT)
+ while (&check->current_step->list != head &&
+ check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+ if (&check->current_step->list == head)
+ break;
+
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);
@@ -2805,9 +2817,13 @@
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
/* bypass all comment rules */
- while (check->current_step->action == TCPCHK_ACT_COMMENT)
+ while (&check->current_step->list != head &&
+ check->current_step->action == TCPCHK_ACT_COMMENT)
check->current_step = (struct tcpcheck_rule *)check->current_step->list.n;
+ if (&check->current_step->list == head)
+ break;
+
if (check->current_step->action == TCPCHK_ACT_EXPECT)
goto tcpcheck_expect;
__conn_data_stop_recv(conn);