src: do not allow to chain more than 16 binops

commit dcb199544563ded462cb7151134278f82a9e6cfd upstream.

netlink_linearize.c has never supported more than 16 chained binops.
Adding more is possible but overwrites the stack in
netlink_gen_bitwise().

Add a recursion counter to catch this at eval stage.

Its not enough to just abort once the counter hits
NFT_MAX_EXPR_RECURSION.

This is because there are valid test cases that exceed this.
For example, evaluation of 1 | 2 will merge the constans, so even
if there are a dozen recursive eval calls this will not end up
with large binop chain post-evaluation.

v2: allow more than 16 binops iff the evaluation function
    did constant-merging.

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

diff --git a/include/expression.h b/include/expression.h
index cd67da454a5a7eb72a6e74bc13f99aa26b6d0c2c..0de2af5485b8cedc07810a4f904dc9d27672d445 100644 (file)
--- a/include/expression.h
+++ b/include/expression.h
@@ -14,6 +14,7 @@
 
 #define NFT_MAX_EXPR_LEN_BYTES (NFT_REG32_COUNT * sizeof(uint32_t))
 #define NFT_MAX_EXPR_LEN_BITS  (NFT_MAX_EXPR_LEN_BYTES * BITS_PER_BYTE)
+#define NFT_MAX_EXPR_RECURSION 16
 
 /**
  * enum expr_types

diff --git a/include/rule.h b/include/rule.h
index 042fd3f0bc73f979f3823598136bd31c9320fa91..fa652f02245550237be89ae7eedee27f36a4bd87 100644 (file)
--- a/include/rule.h
+++ b/include/rule.h
@@ -758,10 +758,13 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc);
  * @rule:      current rule
  * @set:       current set
  * @stmt:      current statement
+ * @stmt_len:  current statement template length
+ * @recursion:  expr evaluation recursion counter
  * @cache:     cache context
  * @debug_mask: debugging bitmask
  * @ectx:      expression context
- * @pctx:      payload context
+ * @_pctx:     payload contexts
+ * @inner_desc: inner header description
  */
 struct eval_ctx {
        struct nft_ctx          *nft;
@@ -772,6 +775,7 @@ struct eval_ctx {
        struct set              *set;
        struct stmt             *stmt;
        uint32_t                stmt_len;
+       uint32_t                recursion;
        struct expr_ctx         ectx;
        struct proto_ctx        pctx;
 };

diff --git a/src/evaluate.c b/src/evaluate.c
index da7ee883691aaa2d05f4de05523bd255634eaf4e..d4ca8dd14c24746825e60407a8fde252d86823fe 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1406,6 +1406,13 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
        struct expr *op = *expr, *left, *right;
        const char *sym = expr_op_symbols[op->op];
        unsigned int max_shift_len = ctx->ectx.len;
+       int ret = -1;
+
+       if (ctx->recursion >= USHRT_MAX)
+               return expr_binary_error(ctx->msgs, op, NULL,
+                                        "Binary operation limit %u reached ",
+                                        ctx->recursion);
+       ctx->recursion++;
 
        if (expr_evaluate(ctx, &op->left) < 0)
                return -1;
@@ -1460,14 +1467,42 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
        switch (op->op) {
        case OP_LSHIFT:
        case OP_RSHIFT:
-               return expr_evaluate_shift(ctx, expr);
+               ret = expr_evaluate_shift(ctx, expr);
+               break;
        case OP_AND:
        case OP_XOR:
        case OP_OR:
-               return expr_evaluate_bitwise(ctx, expr);
+               ret = expr_evaluate_bitwise(ctx, expr);
+               break;
        default:
                BUG("invalid binary operation %u\n", op->op);
        }
+
+
+       if (ctx->recursion == 0)
+               BUG("recursion counter underflow");
+
+       /* can't check earlier: evaluate functions might do constant-merging + expr_free.
+        *
+        * So once we've evaluate everything check for remaining length of the
+        * binop chain.
+        */
+       if (--ctx->recursion == 0) {
+               unsigned int to_linearize = 0;
+
+               op = *expr;
+               while (op && op->etype == EXPR_BINOP && op->left != NULL) {
+                       to_linearize++;
+                       op = op->left;
+
+                       if (to_linearize >= NFT_MAX_EXPR_RECURSION)
+                               return expr_binary_error(ctx->msgs, op, NULL,
+                                                        "Binary operation limit %u reached ",
+                                                        NFT_MAX_EXPR_RECURSION);
+               }
+       }
+
+       return ret;
 }
 
 static int list_member_evaluate(struct eval_ctx *ctx, struct expr **expr)

diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 755ed58ebc5bcb38478f6c2c108b2b1dd6d5898f..d1b798dfcc62065e277ca8c608713c5e0bb336fd 100644 (file)
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -626,10 +626,10 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
                                const struct expr *expr,
                                enum nft_registers dreg)
 {
+       struct expr *binops[NFT_MAX_EXPR_RECURSION];
        struct nftnl_expr *nle;
        struct nft_data_linearize nld;
        struct expr *left, *i;
-       struct expr *binops[16];
        mpz_t mask, xor, val, tmp;
        unsigned int len;
        int n = 0;
@@ -641,8 +641,11 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
 
        binops[n++] = left = (struct expr *) expr;
        while (left->etype == EXPR_BINOP && left->left != NULL &&
-              (left->op == OP_AND || left->op == OP_OR || left->op == OP_XOR))
+              (left->op == OP_AND || left->op == OP_OR || left->op == OP_XOR)) {
+               if (n == array_size(binops))
+                       BUG("NFT_MAX_EXPR_RECURSION limit reached");
                binops[n++] = left = left->left;
+       }
 
        netlink_gen_expr(ctx, binops[--n], dreg);
 

diff --git a/tests/shell/testcases/bogons/nft-f/huge_binop_expr_chain_crash b/tests/shell/testcases/bogons/nft-f/huge_binop_expr_chain_crash
new file mode 100644 (file)
index 0000000..8d1da72
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/huge_binop_expr_chain_crash
@@ -0,0 +1,5 @@
+table t {
+       chain c {
+               meta oifname^a^b^c^d^e^f^g^h^i^j^k^l^m^n^o^p^q^r^s^t^u^v^w^x^y^z^A^B^C^D^E^F^G^H^I^J^K^L^M^N^O^P^Q^R^S^T^U^V^W^X^Y^Z^0^1^2^3^4^5^6^7^8^9 bar
+       }
+}