[nft PATCH] evaluate: fix segfault when adding elements to invalid set

[nft PATCH] evaluate: fix segfault when adding elements to invalid set

[Peter Tirsek]

Adding elements to a set or map with an invalid definition causes nft to
segfault. The following nftables.conf triggers the crash:

    flush ruleset
    create table inet filter
    set inet filter foo {}
    add element inet filter foo { foobar }

Simply parsing and checking the config will trigger it:

    $ nft -c -f nftables.conf.crash
    Segmentation fault

The error in the set/map definition is correctly caught and queued, but
because the set is invalid and does not contain a key type, adding to it
causes a NULL pointer dereference of set->key within setelem_evaluate().

I don't think it's necessary to queue another error since the underlying
problem is correctly detected and reported when parsing the definition
of the set. Simply checking the validity of set->key before using it
seems to fix it, causing the error in the definition of the set to be
reported properly. The element type error isn't caught, but that seems
reasonable since the key type is invalid or unknown anyway:

    $ ./nft -c -f ~/nftables.conf.crash
    /home/pti/nftables.conf.crash:3:21-21: Error: set definition does not specify key
    set inet filter foo {}
                        ^

Signed-off-by: Peter Tirsek <peter@tirsek.com>
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1597
---

As mentioned in the Bugzilla bug, I should note that I'm not familiar
enough with the codebase to have run the testsuite or added a test to
exercise the problem, but I _have_ verified the result manually on the
input listed above. I hope that's okay.

src/evaluate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 82bf1311..073bf871 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3996,6 +3996,9 @@ static int setelem_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
 		return set_not_found(ctx, &ctx->cmd->handle.set.location,
 				     ctx->cmd->handle.set.name);

+	if (set->key == NULL)
+		return -1;
+
 	set->existing_set = set;
 	ctx->set = set;
 	expr_set_context(&ctx->ectx, set->key->dtype, set->key->len);
-- 
2.36.1

Re: [nft PATCH] evaluate: fix segfault when adding elements to invalid set

[Pablo Neira Ayuso]

On Sun, Jun 26, 2022 at 12:47:07AM -0500, Peter Tirsek wrote:
> Adding elements to a set or map with an invalid definition causes nft to
> segfault. The following nftables.conf triggers the crash:
> 
>     flush ruleset
>     create table inet filter
>     set inet filter foo {}
>     add element inet filter foo { foobar }
> 
> Simply parsing and checking the config will trigger it:
> 
>     $ nft -c -f nftables.conf.crash
>     Segmentation fault
> 
> The error in the set/map definition is correctly caught and queued, but
> because the set is invalid and does not contain a key type, adding to it
> causes a NULL pointer dereference of set->key within setelem_evaluate().
> 
> I don't think it's necessary to queue another error since the underlying
> problem is correctly detected and reported when parsing the definition
> of the set. Simply checking the validity of set->key before using it
> seems to fix it, causing the error in the definition of the set to be
> reported properly. The element type error isn't caught, but that seems
> reasonable since the key type is invalid or unknown anyway:
> 
>     $ ./nft -c -f ~/nftables.conf.crash
>     /home/pti/nftables.conf.crash:3:21-21: Error: set definition does not specify key
>     set inet filter foo {}
>                         ^

Applied, thanks