BUG/MINOR: stick-table: reject negative stick counter IDs in sc-* actions

The parsers of sc-inc-gpc, sc-inc-gpc0/1, sc-add-gpc, sc-set-gpt and
sc-set-gpt0 read the stick counter ID with strtol() into a signed int and
only check it against tune.stick-counters. A negative ID such as in
"sc-inc-gpc(0,-2)" or "sc-set-gpt0(-1)" is thus accepted, and at runtime
the actions read &s->stkctr[-2] or &sess->stkctr[-1], before the start of
the stick counters array. ASan reports a heap-buffer-overflow READ in
stkctr_entry() called from the action, and a release build may crash or
use whatever lies there as a stick-table entry pointer to update it.

The sc_* sample fetches are not affected since they convert the ID to an
unsigned int before the check, and track-sc* parses an unsigned number.

Let's also reject negative IDs. A reg-test is added.

This should be backported to all stable branches. sc-add-gpc() appeared
in 2.8 and the others are older.

diff --git a/src/stick_table.c b/src/stick_table.c
index 9c447bb99f73a960c2feecbf04158dd5a01e4452..dbb59f7e0822b686d890112bceb9f25d0fb3e82b 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -2914,7 +2914,7 @@ static enum act_parse_ret parse_inc_gpc(const char **args, int *arg, struct prox
                                return ACT_RET_PRS_ERR;
                        }
 
-                       if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+                       if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
                                memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d (tune.stick-counters)",
                                          args[*arg-1], global.tune.nb_stk_ctr-1);
                                return ACT_RET_PRS_ERR;
@@ -2942,7 +2942,7 @@ static enum act_parse_ret parse_inc_gpc(const char **args, int *arg, struct prox
                                return ACT_RET_PRS_ERR;
                        }
 
-                       if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+                       if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
                                memprintf(err, "invalid stick table track ID. The max allowed ID is %d (tune.stick-counters)",
                                          global.tune.nb_stk_ctr-1);
                                return ACT_RET_PRS_ERR;
@@ -3144,7 +3144,7 @@ static enum act_parse_ret parse_set_gpt(const char **args, int *arg, struct prox
                                return ACT_RET_PRS_ERR;
                        }
 
-                       if (rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
+                       if (rule->arg.gpt.sc < 0 || rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
                                memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d",
                                          args[*arg-1], global.tune.nb_stk_ctr-1);
                                return ACT_RET_PRS_ERR;
@@ -3170,7 +3170,7 @@ static enum act_parse_ret parse_set_gpt(const char **args, int *arg, struct prox
                                return ACT_RET_PRS_ERR;
                        }
 
-                       if (rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
+                       if (rule->arg.gpt.sc < 0 || rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
                                memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d",
                                          args[*arg-1], global.tune.nb_stk_ctr-1);
                                return ACT_RET_PRS_ERR;
@@ -3337,7 +3337,7 @@ static enum act_parse_ret parse_add_gpc(const char **args, int *arg, struct prox
                        return ACT_RET_PRS_ERR;
                }
 
-               if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+               if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
                        memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d (tune.stick-counters)",
                                  args[*arg-1], global.tune.nb_stk_ctr - 1);
                        return ACT_RET_PRS_ERR;