Sometimes nftables will segfault when doing error-unwind of the included afl-generated bogon. The problem is the unconditional write access to expr->set_flags in expr_evaluate_map(): mappings->set_flags |= NFT_SET_MAP; ... but mappings can point to EXPR_VARIABLE (legal), where this will flip a bit in unused, but allocated memory (i.e., has no effect). In case of the bogon, mapping is EXPR_RANGE_SYMBOL, and the store can flip a bit in identifier_range[1], this causes crash when the pointer is freed. We can't use expr->set_flags unconditionally, so rework this to pass set_flags as argument and place all read and write accesses in places where we've made sure we are dealing with EXPR_SET. Signed-off-by: Florian Westphal <fw@xxxxxxxxx> --- src/evaluate.c | 29 ++++++++++++------- .../bogons/nft-f/range_expression_corruption | 2 ++ 2 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 tests/shell/testcases/bogons/nft-f/range_expression_corruption diff --git a/src/evaluate.c b/src/evaluate.c index 722c11a23c2d..7fc210fd3b12 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -123,16 +123,16 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx, struct set *set; struct handle h; - if (set_is_datamap(expr->set_flags)) + if (set_is_datamap(flags)) key_fix_dtype_byteorder(key); set = set_alloc(&expr->location); - set->flags = expr->set_flags | flags; + set->flags = flags; set->handle.set.name = xstrdup(name); set->key = key; set->data = data; set->init = expr; - set->automerge = set->flags & NFT_SET_INTERVAL; + set->automerge = flags & NFT_SET_INTERVAL; handle_merge(&set->handle, &ctx->cmd->handle); @@ -2117,6 +2117,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) { struct expr *map = *expr, *mappings; struct expr_ctx ectx = ctx->ectx; + uint32_t set_flags = NFT_SET_MAP; struct expr *key, *data; if (map->map->etype == EXPR_CT && @@ -2145,11 +2146,14 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) ctx->stmt_len = 0; mappings = map->mappings; - mappings->set_flags |= NFT_SET_MAP; switch (map->mappings->etype) { - case EXPR_VARIABLE: + case EXPR_CONCAT: + case EXPR_LIST: case EXPR_SET: + set_flags |= mappings->set_flags; + /* fallthrough */ + case EXPR_VARIABLE: if (ctx->ectx.key && ctx->ectx.key->etype == EXPR_CONCAT) { key = expr_clone(ctx->ectx.key); } else { @@ -2179,7 +2183,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) mappings = implicit_set_declaration(ctx, "__map%d", key, data, mappings, - NFT_SET_ANONYMOUS); + NFT_SET_ANONYMOUS | set_flags); if (!mappings) return -1; @@ -2807,7 +2811,7 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) implicit_set_declaration(ctx, "__set%d", expr_get(left), NULL, right, - NFT_SET_ANONYMOUS); + right->set_flags | NFT_SET_ANONYMOUS); if (!right) return -1; @@ -3529,7 +3533,8 @@ static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt) set->set_flags |= NFT_SET_EVAL; setref = implicit_set_declaration(ctx, stmt->meter.name, - expr_get(key), NULL, set, 0); + expr_get(key), NULL, set, + NFT_SET_EVAL | set->set_flags); if (setref) setref->set->desc.size = stmt->meter.size; } @@ -4742,6 +4747,7 @@ static int stmt_evaluate_map(struct eval_ctx *ctx, struct stmt *stmt) static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt) { struct expr *map = stmt->objref.expr; + uint32_t set_flags = NFT_SET_OBJECT; struct expr *mappings; struct expr *key; @@ -4753,11 +4759,12 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt) "Map expression can not be constant"); mappings = map->mappings; - mappings->set_flags |= NFT_SET_OBJECT; switch (map->mappings->etype) { - case EXPR_VARIABLE: case EXPR_SET: + set_flags |= mappings->set_flags; + /* fallthrough */ + case EXPR_VARIABLE: key = constant_expr_alloc(&stmt->location, ctx->ectx.dtype, ctx->ectx.byteorder, @@ -4765,7 +4772,7 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt) mappings = implicit_set_declaration(ctx, "__objmap%d", key, NULL, mappings, - NFT_SET_ANONYMOUS); + set_flags | NFT_SET_ANONYMOUS); if (!mappings) return -1; mappings->set->objtype = stmt->objref.type; diff --git a/tests/shell/testcases/bogons/nft-f/range_expression_corruption b/tests/shell/testcases/bogons/nft-f/range_expression_corruption new file mode 100644 index 000000000000..b77221bd11a4 --- /dev/null +++ b/tests/shell/testcases/bogons/nft-f/range_expression_corruption @@ -0,0 +1,2 @@ +aal tht@nh,32,3 set ctag| oi to ip + p sept ct l3proto map q -u dscp | ma \ No newline at end of file -- 2.45.3