[PATCH nft] cache: don't crash when filter is NULL

[PATCH nft] cache: don't crash when filter is NULL

[Florian Westphal]

a delete request will cause a crash in obj_cache_dump, move the deref
into the filter block.

Fixes: dbff26bfba83 ("cache: consolidate reset command")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/cache.c                                                 | 6 ++++--
 .../testcases/bogons/nft-f/delete_nonexistant_object_crash  | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash

diff --git a/src/cache.c b/src/cache.c
index b75a5bf3283c..c0d96bd14a80 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -902,6 +902,7 @@ static struct nftnl_obj_list *obj_cache_dump(struct netlink_ctx *ctx,
 	int family = NFPROTO_UNSPEC;
 	const char *table = NULL;
 	const char *obj = NULL;
+	bool reset = false;
 	bool dump = true;
 
 	if (filter) {
@@ -914,9 +915,10 @@ static struct nftnl_obj_list *obj_cache_dump(struct netlink_ctx *ctx,
 		}
 		if (filter->list.obj_type)
 			type = filter->list.obj_type;
+
+		reset = filter->reset.obj;
 	}
-	obj_list = mnl_nft_obj_dump(ctx, family, table, obj, type, dump,
-				    filter->reset.obj);
+	obj_list = mnl_nft_obj_dump(ctx, family, table, obj, type, dump, reset);
 	if (!obj_list) {
                 if (errno == EINTR)
 			return NULL;
diff --git a/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash b/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash
new file mode 100644
index 000000000000..c369dec8c07d
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash
@@ -0,0 +1 @@
+delete quota a b
-- 
2.49.0

Re: [PATCH nft] cache: don't crash when filter is NULL

[Pablo Neira Ayuso]

On Tue, Apr 01, 2025 at 04:29:14PM +0200, Florian Westphal wrote:
> a delete request will cause a crash in obj_cache_dump, move the deref
> into the filter block.
> 
> Fixes: dbff26bfba83 ("cache: consolidate reset command")
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks.

> ---
>  src/cache.c                                                 | 6 ++++--
>  .../testcases/bogons/nft-f/delete_nonexistant_object_crash  | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>  create mode 100644 tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash
> 
> diff --git a/src/cache.c b/src/cache.c
> index b75a5bf3283c..c0d96bd14a80 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -902,6 +902,7 @@ static struct nftnl_obj_list *obj_cache_dump(struct netlink_ctx *ctx,
>  	int family = NFPROTO_UNSPEC;
>  	const char *table = NULL;
>  	const char *obj = NULL;
> +	bool reset = false;
>  	bool dump = true;
>  
>  	if (filter) {
> @@ -914,9 +915,10 @@ static struct nftnl_obj_list *obj_cache_dump(struct netlink_ctx *ctx,
>  		}
>  		if (filter->list.obj_type)
>  			type = filter->list.obj_type;
> +
> +		reset = filter->reset.obj;
>  	}
> -	obj_list = mnl_nft_obj_dump(ctx, family, table, obj, type, dump,
> -				    filter->reset.obj);
> +	obj_list = mnl_nft_obj_dump(ctx, family, table, obj, type, dump, reset);
>  	if (!obj_list) {
>                  if (errno == EINTR)
>  			return NULL;
> diff --git a/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash b/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash
> new file mode 100644
> index 000000000000..c369dec8c07d
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-f/delete_nonexistant_object_crash
> @@ -0,0 +1 @@
> +delete quota a b
> -- 
> 2.49.0
> 
> 

Re: [PATCH nft] cache: don't crash when filter is NULL

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On Tue, Apr 01, 2025 at 04:33:42PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Apr 01, 2025 at 04:29:14PM +0200, Florian Westphal wrote:
> > a delete request will cause a crash in obj_cache_dump, move the deref
> > into the filter block.
> > 
> > Fixes: dbff26bfba83 ("cache: consolidate reset command")
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> 
> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

BTW. Same pattern in:

- rule_cache_dump()

Maybe collapse this chunk too?

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 800 bytes --]

diff --git a/src/cache.c b/src/cache.c
index b75a5bf3283c..52f7c9abd741 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -714,6 +714,7 @@ static int rule_cache_dump(struct netlink_ctx *ctx, const struct handle *h,
 	const char *chain = NULL;
 	uint64_t rule_handle = 0;
 	int family = h->family;
+	bool reset = false;
 	bool dump = true;
 
 	if (filter) {
@@ -727,11 +728,12 @@ static int rule_cache_dump(struct netlink_ctx *ctx, const struct handle *h,
 		}
 		if (filter->list.family)
 			family = filter->list.family;
+
+		reset = filter->reset.rule;
 	}
 
 	rule_cache = mnl_nft_rule_dump(ctx, family,
-				       table, chain, rule_handle, dump,
-				       filter->reset.rule);
+				       table, chain, rule_handle, dump, reset);
 	if (rule_cache == NULL) {
 		if (errno == EINTR)
 			return -1;