[PATCH] src: fix deref of null.ret in rule.c

[PATCH] src: fix deref of null.ret in rule.c

[Anton Moryakov]

Fix potential null pointer dereference in `do_list_flowtable`.

The pointer `table` is initialized to NULL and passed to `do_list_flowtable`,
where it may be dereferenced. This can lead to a crash if `table` remains NULL.

Changes:
- Added a NULL check for the `table` pointer before calling `do_list_flowtable`.
- Return an error code (-1) if `table` is NULL to handle the case where the table is not found.

Triggers found by static analyzer Svace.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>

---
 src/rule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index f7582914..59d3f3ac 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1556,7 +1556,7 @@ static int do_delete_setelems(struct netlink_ctx *ctx, struct cmd *cmd)
 	const struct set *set = cmd->elem.set;
 	struct expr *expr = cmd->elem.expr;
 
-	if (set_is_non_concat_range(set) &&
+	if (set && set_is_non_concat_range(set) &&
 	    set_to_intervals(set, expr, false) < 0)
 		return -1;
 
-- 
2.30.2

Re: [PATCH] src: fix deref of null.ret in rule.c

[Florian Westphal]

Anton Moryakov <ant.v.moryakov@gmail.com> wrote:
> Fix potential null pointer dereference in `do_list_flowtable`.
> 
> The pointer `table` is initialized to NULL and passed to `do_list_flowtable`,
> where it may be dereferenced. This can lead to a crash if `table` remains NULL.
> 
> Changes:
> - Added a NULL check for the `table` pointer before calling `do_list_flowtable`.
> - Return an error code (-1) if `table` is NULL to handle the case where the table is not found.

This changelog doesn't match the patch.

> index f7582914..59d3f3ac 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1556,7 +1556,7 @@ static int do_delete_setelems(struct netlink_ctx *ctx, struct cmd *cmd)
>  	const struct set *set = cmd->elem.set;
>  	struct expr *expr = cmd->elem.expr;
>  
> -	if (set_is_non_concat_range(set) &&
> +	if (set && set_is_non_concat_range(set) &&
>  	    set_to_intervals(set, expr, false) < 0)
>  		return -1;

You need to explain how "set" can be NULL here.

This gets allocated in nft_cmd_expand, where set was
already dereferenced.