evaluate: restrict allowed subtypes of concatenations

We need to restrict this, included bogon asserts with:
BUG: unknown expression type prefix
nft: src/netlink_linearize.c:940: netlink_gen_expr: Assertion `0' failed.

Prefix expressions are only allowed if the concatenation is used within
a set element, not when specifying the lookup key.

For the former, anything that represents a value is allowed.
For the latter, only what will generate data (fill a register) is
permitted.

At this time we do not have an annotation that tells if the expression
is on the left hand side (lookup key) or right hand side (set element).

Add a new list recursion counter for this. If its 0 then we're building
the lookup key, if its the latter the concatenation is the RHS part
of a relational expression and prefix, ranges and so on are allowed.

IOW, we don't really need a recursion counter, another type of annotation
that would tell if the expression is placed on the left or right hand side
of another expression would work too.

v2: explicitly list all 'illegal' expression types instead of
using a default label for them.

This will raise a compiler warning to remind us to adjust the case
labels in case a new expression type gets added in the future.

Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/include/rule.h b/include/rule.h
index e7e80a41506cb0b796dff0e3a6f3781eeddc4815..655d6abaf5fae86051fe3f68a9119d238e61b636 100644 (file)
--- a/include/rule.h
+++ b/include/rule.h
@@ -758,6 +758,7 @@ extern void cmd_free(struct cmd *cmd);
 
 struct eval_recursion {
        uint16_t binop;
+       uint16_t list;
 };
 
 /**

diff --git a/src/evaluate.c b/src/evaluate.c
index 77bdb9cd74c5a977c1fd2c0ae854e7aeae79ec0c..cc35f88417bc53abcb6d7bd86ac9b7e0f3ee8650 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1640,10 +1640,18 @@ static int list_member_evaluate(struct eval_ctx *ctx, struct expr **expr)
        struct expr *next = list_entry((*expr)->list.next, struct expr, list);
        int err;
 
+       /* should never be hit in practice */
+       if (ctx->recursion.list >= USHRT_MAX)
+               return expr_binary_error(ctx->msgs, next, NULL,
+                                        "List limit %u reached ",
+                                        ctx->recursion.list);
+
+       ctx->recursion.list++;
        assert(*expr != next);
        list_del(&(*expr)->list);
        err = expr_evaluate(ctx, expr);
        list_add_tail(&(*expr)->list, &next->list);
+       ctx->recursion.list--;
        return err;
 }
 
@@ -1706,10 +1714,61 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
                if (list_member_evaluate(ctx, &i) < 0)
                        return -1;
 
-               if (i->etype == EXPR_SET)
+               switch (i->etype) {
+               case EXPR_INVALID:
+               case __EXPR_MAX:
+                       BUG("Unexpected etype %d", i->etype);
+                       break;
+               case EXPR_VALUE:
+               case EXPR_UNARY:
+               case EXPR_BINOP:
+               case EXPR_RELATIONAL:
+               case EXPR_CONCAT:
+               case EXPR_MAP:
+               case EXPR_PAYLOAD:
+               case EXPR_EXTHDR:
+               case EXPR_META:
+               case EXPR_RT:
+               case EXPR_CT:
+               case EXPR_SET_ELEM:
+               case EXPR_NUMGEN:
+               case EXPR_HASH:
+               case EXPR_FIB:
+               case EXPR_SOCKET:
+               case EXPR_OSF:
+               case EXPR_XFRM:
+                       break;
+               case EXPR_RANGE:
+               case EXPR_PREFIX:
+                       /* allowed on RHS (e.g. th dport . mark { 1-65535 . 42 }
+                        *                                       ~~~~~~~~ allowed
+                        * but not on LHS (e.g  1-4 . mark { ...}
+                        *                      ~~~ illegal
+                        *
+                        * recursion.list > 0 means that the concatenation is
+                        * part of another expression, such as EXPR_MAPPING or
+                        * EXPR_SET_ELEM (is used as RHS).
+                        */
+                       if (ctx->recursion.list > 0)
+                               break;
+
+                       return expr_error(ctx->msgs, i,
+                                         "cannot use %s in concatenation",
+                                         expr_name(i));
+               case EXPR_VERDICT:
+               case EXPR_SYMBOL:
+               case EXPR_VARIABLE:
+               case EXPR_LIST:
+               case EXPR_SET:
+               case EXPR_SET_REF:
+               case EXPR_MAPPING:
+               case EXPR_SET_ELEM_CATCHALL:
+               case EXPR_RANGE_VALUE:
+               case EXPR_RANGE_SYMBOL:
                        return expr_error(ctx->msgs, i,
                                          "cannot use %s in concatenation",
                                          expr_name(i));
+               }
 
                if (!i->dtype)
                        return expr_error(ctx->msgs, i,

diff --git a/tests/shell/testcases/bogons/nft-f/unknown_expression_type_prefix_assert b/tests/shell/testcases/bogons/nft-f/unknown_expression_type_prefix_assert
new file mode 100644 (file)
index 0000000..d7f8526
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/unknown_expression_type_prefix_assert
@@ -0,0 +1,9 @@
+table t {
+       set sc {
+               type inet_service . ifname
+       }
+
+       chain c {
+               tcp dport . bla* @sc accept
+       }
+}