From: Florian Westphal <fw@strlen.de>
Date: Tue, 24 Jun 2025 21:01:13 +0000 (+0200)
Subject: evaluate: make sure chain jump name comes with a null byte
X-Git-Tag: v1.0.6.1~40
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=535689d466f46ead3ff4377da58668e6e614983e;p=thirdparty%2Fnftables.git

evaluate: make sure chain jump name comes with a null byte

commit ca0c49d1bdb944534851c3dcb4c8ce16f1675074 upstream.

There is a stack oob read access in netlink_gen_chain():

	mpz_export_data(chain, expr->chain->value,
			BYTEORDER_HOST_ENDIAN, len);
	snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain);

There is no guarantee that chain[] is null terminated, so snprintf
can read past chain[] array.  ASAN report is:

AddressSanitizer: stack-buffer-overflow on address 0x7ffff5f00520 at ..
READ of size 257 at 0x7ffff5f00520 thread T0
    #0 0x00000032ffb6 in printf_common(void*, char const*, __va_list_tag*) (src/nft+0x32ffb6)
    #1 0x00000033055d in vsnprintf (src/nft+0x33055d)
    #2 0x000000332071 in snprintf (src/nft+0x332071)
    #3 0x0000004eef03 in netlink_gen_chain src/netlink.c:454:2
    #4 0x0000004eef03 in netlink_gen_verdict src/netlink.c:467:4

Reject chain jumps that exceed 255 characters, which matches the netlink
policy on the kernel side.

The included reproducer fails without asan too because the kernel will
reject the too-long chain name. But that happens after the asan detected
bogus read.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
---

diff --git a/src/evaluate.c b/src/evaluate.c
index 8fedb9b7..72baf585 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2863,16 +2863,32 @@ static int expr_evaluate_flagcmp(struct eval_ctx *ctx, struct expr **exprp)
 	return expr_evaluate(ctx, exprp);
 }
 
-static int verdict_validate_chainlen(struct eval_ctx *ctx,
+static int verdict_validate_chain(struct eval_ctx *ctx,
 				     struct expr *chain)
 {
-	if (chain->len > NFT_CHAIN_MAXNAMELEN * BITS_PER_BYTE)
+	char buf[NFT_CHAIN_MAXNAMELEN];
+	unsigned int len;
+
+	len = chain->len / BITS_PER_BYTE;
+	if (len > NFT_CHAIN_MAXNAMELEN)
 		return expr_error(ctx->msgs, chain,
 				  "chain name too long (%u, max %u)",
 				  chain->len / BITS_PER_BYTE,
 				  NFT_CHAIN_MAXNAMELEN);
 
-	return 0;
+	if (!len)
+		return expr_error(ctx->msgs, chain,
+				  "chain name length 0 not allowed");
+
+	memset(buf, 0, sizeof(buf));
+	mpz_export_data(buf, chain->value, BYTEORDER_HOST_ENDIAN, len);
+
+	if (strnlen(buf, sizeof(buf)) < sizeof(buf))
+		return 0;
+
+	return expr_error(ctx->msgs, chain,
+			  "chain name must be smaller than %u",
+			  NFT_CHAIN_MAXNAMELEN);
 }
 
 static int expr_evaluate_verdict(struct eval_ctx *ctx, struct expr **exprp)
@@ -2883,7 +2899,7 @@ static int expr_evaluate_verdict(struct eval_ctx *ctx, struct expr **exprp)
 	case NFT_GOTO:
 	case NFT_JUMP:
 		if (expr->chain->etype == EXPR_VALUE &&
-		    verdict_validate_chainlen(ctx, expr->chain))
+		    verdict_validate_chain(ctx, expr->chain))
 			return -1;
 
 		break;
@@ -3115,7 +3131,7 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
 						  "not a value expression");
 			}
 
-			if (verdict_validate_chainlen(ctx, stmt->expr->chain))
+			if (verdict_validate_chain(ctx, stmt->expr->chain))
 				return -1;
 		}
 		break;
diff --git a/tests/shell/testcases/bogons/nft-f/asan_out_of_bounds_read_with_long_chain b/tests/shell/testcases/bogons/nft-f/asan_out_of_bounds_read_with_long_chain
new file mode 100644
index 00000000..e166a030
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/asan_out_of_bounds_read_with_long_chain
@@ -0,0 +1,3 @@
+add table t
+add chain t eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+add rule  t eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee jump   eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee