From dcb199544563ded462cb7151134278f82a9e6cfd Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Thu, 21 Dec 2023 11:25:14 +0100
Subject: [PATCH] src: do not allow to chain more than 16 binops

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>
---
 include/expression.h                          |  1 +
 include/rule.h                                |  6 ++-
 src/evaluate.c                                | 39 ++++++++++++++++++-
 src/netlink_linearize.c                       |  7 +++-
 .../bogons/nft-f/huge_binop_expr_chain_crash  |  5 +++
 5 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/huge_binop_expr_chain_crash

diff --git a/include/expression.h b/include/expression.h
index 809089c8..f5d7e160 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -13,6 +13,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 6236d292..6835fe06 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -753,10 +753,13 @@ extern void cmd_free(struct cmd *cmd);
  * @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;
@@ -767,6 +770,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[2];
 	const struct proto_desc	*inner_desc;
diff --git a/src/evaluate.c b/src/evaluate.c
index 26f0110f..41524eef 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1418,6 +1418,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;
@@ -1472,14 +1479,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 d8b41a08..50dbd36c 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -695,10 +695,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;
@@ -710,8 +710,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;
+	}
 	n--;
 
 	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
index 00000000..8d1da726
--- /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
+	}
+}
-- 
2.33.0