[nft PATCH] json: Fix for memleak in __binop_expr_json

[nft PATCH] json: Fix for memleak in __binop_expr_json

[Phil Sutter]

When merging the JSON arrays generated for LHS and RHS of nested binop
expressions, the emptied array objects leak if their reference is not
decremented.

Fix this and tidy up other spots which did it right already by
introducing a json_array_extend wrapper.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Fixes: 0ac39384fd9e4 ("json: Accept more than two operands in binary expressions")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/json.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/json.c b/src/json.c
index 3753017169930..b4fad0abd4b35 100644
--- a/src/json.c
+++ b/src/json.c
@@ -42,6 +42,15 @@
 })
 #endif
 
+static int json_array_extend_new(json_t *array, json_t *other_array)
+{
+	int ret;
+
+	ret = json_array_extend(array, other_array);
+	json_decref(other_array);
+	return ret;
+}
+
 static json_t *expr_print_json(const struct expr *expr, struct output_ctx *octx)
 {
 	const struct expr_ops *ops;
@@ -546,8 +555,10 @@ __binop_expr_json(int op, const struct expr *expr, struct output_ctx *octx)
 	json_t *a = json_array();
 
 	if (expr->etype == EXPR_BINOP && expr->op == op) {
-		json_array_extend(a, __binop_expr_json(op, expr->left, octx));
-		json_array_extend(a, __binop_expr_json(op, expr->right, octx));
+		json_array_extend_new(a,
+				      __binop_expr_json(op, expr->left, octx));
+		json_array_extend_new(a,
+				      __binop_expr_json(op, expr->right, octx));
 	} else {
 		json_array_append_new(a, expr_print_json(expr, octx));
 	}
@@ -1743,8 +1754,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
 		}
 	}
 
-	json_array_extend(root, rules);
-	json_decref(rules);
+	json_array_extend_new(root, rules);
 
 	return root;
 }
@@ -1752,7 +1762,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
 static json_t *do_list_ruleset_json(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	unsigned int family = cmd->handle.family;
-	json_t *root = json_array(), *tmp;
+	json_t *root = json_array();
 	struct table *table;
 
 	list_for_each_entry(table, &ctx->nft->cache.table_cache.list, cache.list) {
@@ -1760,9 +1770,7 @@ static json_t *do_list_ruleset_json(struct netlink_ctx *ctx, struct cmd *cmd)
 		    table->handle.family != family)
 			continue;
 
-		tmp = table_print_json_full(ctx, table);
-		json_array_extend(root, tmp);
-		json_decref(tmp);
+		json_array_extend_new(root, table_print_json_full(ctx, table));
 	}
 
 	return root;
-- 
2.43.0

Re: [nft PATCH] json: Fix for memleak in __binop_expr_json

[Pablo Neira Ayuso]

On Wed, Apr 24, 2024 at 11:58:21PM +0200, Phil Sutter wrote:
> When merging the JSON arrays generated for LHS and RHS of nested binop
> expressions, the emptied array objects leak if their reference is not
> decremented.
> 
> Fix this and tidy up other spots which did it right already by
> introducing a json_array_extend wrapper.

Thanks for fixing it up so quick, no more issues with tests/shell.

> Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Fixes: 0ac39384fd9e4 ("json: Accept more than two operands in binary expressions")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Re: [nft PATCH] json: Fix for memleak in __binop_expr_json

[Phil Sutter]

On Thu, Apr 25, 2024 at 12:12:43AM +0200, Pablo Neira Ayuso wrote:
> On Wed, Apr 24, 2024 at 11:58:21PM +0200, Phil Sutter wrote:
> > When merging the JSON arrays generated for LHS and RHS of nested binop
> > expressions, the emptied array objects leak if their reference is not
> > decremented.
> > 
> > Fix this and tidy up other spots which did it right already by
> > introducing a json_array_extend wrapper.
> 
> Thanks for fixing it up so quick, no more issues with tests/shell.

Thanks for verifying, patch applied.