[PATCH nft 1/2] src: add rule_stmt_insert_at() and use it

[PATCH nft 1/2] src: add rule_stmt_insert_at() and use it

[Pablo Neira Ayuso]

This helper function adds a statement at a given position and it updates
the rule statement counter.

This patch fixes this:

flush table bridge test-bridge
add rule bridge test-bridge input vlan id 1 ip saddr 10.0.0.1
rule.c:2870:5: runtime error: index 2 out of bounds for type 'stmt *[*]'
=================================================================
==1043==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdd69c1350 at pc 0x7f1036f53330 bp 0x7ffdd69c1300 sp 0x7ffdd69c12f8
WRITE of size 8 at 0x7ffdd69c1350 thread T0
    #0 0x7f1036f5332f in payload_try_merge /home/mbr/nftables/src/rule.c:2870
    #1 0x7f1036f534b7 in rule_postprocess /home/mbr/nftables/src/rule.c:2885
    #2 0x7f1036fb2785 in rule_evaluate /home/mbr/nftables/src/evaluate.c:3744
    #3 0x7f1036fb627b in cmd_evaluate_add /home/mbr/nftables/src/evaluate.c:3982
    #4 0x7f1036fbb9e9 in cmd_evaluate /home/mbr/nftables/src/evaluate.c:4462
    #5 0x7f10370652d2 in nft_evaluate /home/mbr/nftables/src/libnftables.c:414
    #6 0x7f1037065ba1 in nft_run_cmd_from_buffer /home/mbr/nftables/src/libnftables.c:447

Reported-by: Michael Braun <michael-dev@fami-braun.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h | 3 +++
 src/evaluate.c | 9 +++++----
 src/rule.c     | 7 +++++++
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index ac69b30673e8..5311b5630165 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -280,6 +280,9 @@ extern void rule_print(const struct rule *rule, struct output_ctx *octx);
 extern struct rule *rule_lookup(const struct chain *chain, uint64_t handle);
 extern struct rule *rule_lookup_by_index(const struct chain *chain,
 					 uint64_t index);
+void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
+			 struct stmt *stmt);
+
 
 /**
  * struct set - nftables set
diff --git a/src/evaluate.c b/src/evaluate.c
index 597141317000..4cf28987049b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -659,7 +659,7 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 		if (err < 0)
 			return err;
 
-		list_add_tail(&nstmt->list, &ctx->stmt->list);
+		rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 	}
 
 	assert(base <= PROTO_BASE_MAX);
@@ -673,7 +673,7 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 		return 1;
 
 	payload->payload.offset += ctx->pctx.protocol[base].offset;
-	list_add_tail(&nstmt->list, &ctx->stmt->list);
+	rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 
 	return 0;
 }
@@ -698,7 +698,8 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 	if (desc == NULL) {
 		if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
 			return -1;
-		list_add_tail(&nstmt->list, &ctx->stmt->list);
+
+		rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 	} else {
 		/* No conflict: Same payload protocol as context, adjust offset
 		 * if needed.
@@ -840,8 +841,8 @@ static int ct_gen_nh_dependency(struct eval_ctx *ctx, struct expr *ct)
 	relational_expr_pctx_update(&ctx->pctx, dep);
 
 	nstmt = expr_stmt_alloc(&dep->location, dep);
+	rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 
-	list_add_tail(&nstmt->list, &ctx->stmt->list);
 	return 0;
 }
 
diff --git a/src/rule.c b/src/rule.c
index 23b1cbfc8fb2..0759bec5f1a0 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -686,6 +686,13 @@ struct rule *rule_lookup_by_index(const struct chain *chain, uint64_t index)
 	return NULL;
 }
 
+void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
+			 struct stmt *stmt)
+{
+	list_add_tail(&nstmt->list, &stmt->list);
+	rule->num_stmts++;
+}
+
 struct scope *scope_alloc(void)
 {
 	struct scope *scope = xzalloc(sizeof(struct scope));
-- 
2.20.1

[PATCH nft 2/2] src: add rule_stmt_append() and use it

[Pablo Neira Ayuso]

This helper function adds a statement at the end of the rule statement
list and it updates the rule statement counter.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h            | 1 +
 src/netlink_delinearize.c | 7 +++----
 src/parser_json.c         | 6 ++----
 src/rule.c                | 6 ++++++
 src/xt.c                  | 4 ++--
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 5311b5630165..1a4ec3d8bc37 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -280,6 +280,7 @@ extern void rule_print(const struct rule *rule, struct output_ctx *octx);
 extern struct rule *rule_lookup(const struct chain *chain, uint64_t handle);
 extern struct rule *rule_lookup_by_index(const struct chain *chain,
 					 uint64_t index);
+void rule_stmt_append(struct rule *rule, struct stmt *stmt);
 void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
 			 struct stmt *stmt);
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index f721d15c330f..7f7ad2626e14 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -563,8 +563,7 @@ static void netlink_parse_payload_stmt(struct netlink_parse_ctx *ctx,
 	payload_init_raw(expr, base, offset, len);
 
 	stmt = payload_stmt_alloc(loc, expr, val);
-
-	list_add_tail(&stmt->list, &ctx->rule->stmts);
+	rule_stmt_append(ctx->rule, stmt);
 }
 
 static void netlink_parse_payload(struct netlink_parse_ctx *ctx,
@@ -615,7 +614,7 @@ static void netlink_parse_exthdr(struct netlink_parse_ctx *ctx,
 		expr_set_type(val, expr->dtype, expr->byteorder);
 
 		stmt = exthdr_stmt_alloc(loc, expr, val);
-		list_add_tail(&stmt->list, &ctx->rule->stmts);
+		rule_stmt_append(ctx->rule, stmt);
 	}
 }
 
@@ -1672,7 +1671,7 @@ static int netlink_parse_rule_expr(struct nftnl_expr *nle, void *arg)
 	if (err < 0)
 		return err;
 	if (ctx->stmt != NULL) {
-		list_add_tail(&ctx->stmt->list, &ctx->rule->stmts);
+		rule_stmt_append(ctx->rule, ctx->stmt);
 		ctx->stmt = NULL;
 	}
 	return 0;
diff --git a/src/parser_json.c b/src/parser_json.c
index a1765027fdf3..4468407b0ecd 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2731,8 +2731,7 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 			return NULL;
 		}
 
-		rule->num_stmts++;
-		list_add_tail(&stmt->list, &rule->stmts);
+		rule_stmt_append(rule, stmt);
 	}
 
 	if (op == CMD_ADD)
@@ -3404,8 +3403,7 @@ static struct cmd *json_parse_cmd_replace(struct json_ctx *ctx,
 			return NULL;
 		}
 
-		rule->num_stmts++;
-		list_add_tail(&stmt->list, &rule->stmts);
+		rule_stmt_append(rule, stmt);
 	}
 
 	if (op == CMD_REPLACE)
diff --git a/src/rule.c b/src/rule.c
index 0759bec5f1a0..c58aa359259e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -686,6 +686,12 @@ struct rule *rule_lookup_by_index(const struct chain *chain, uint64_t index)
 	return NULL;
 }
 
+void rule_stmt_append(struct rule *rule, struct stmt *stmt)
+{
+	list_add_tail(&stmt->list, &rule->stmts);
+	rule->num_stmts++;
+}
+
 void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
 			 struct stmt *stmt)
 {
diff --git a/src/xt.c b/src/xt.c
index b0f5a30c46b5..f39acf30275a 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -238,7 +238,7 @@ void netlink_parse_match(struct netlink_parse_ctx *ctx,
 	stmt->xt.name = strdup(name);
 	stmt->xt.type = NFT_XT_MATCH;
 #endif
-	list_add_tail(&stmt->list, &ctx->rule->stmts);
+	rule_stmt_append(ctx->rule, stmt);
 }
 
 void netlink_parse_target(struct netlink_parse_ctx *ctx,
@@ -283,7 +283,7 @@ void netlink_parse_target(struct netlink_parse_ctx *ctx,
 	stmt->xt.name = strdup(name);
 	stmt->xt.type = NFT_XT_TARGET;
 #endif
-	list_add_tail(&stmt->list, &ctx->rule->stmts);
+	rule_stmt_append(ctx->rule, stmt);
 }
 
 #ifdef HAVE_LIBXTABLES
-- 
2.20.1