178 lines
5.6 KiB
Diff
178 lines
5.6 KiB
Diff
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
|
|
|