[nft PATCH] datatype: fix ether address parsing of integer values

[nft PATCH] datatype: fix ether address parsing of integer values

[Brian Witte]

When parsing "ether daddr 0x64", integer_expr converts the hex value
to decimal string "100". lladdr_type_parse then interprets this as
hex, yielding 0x100 = 256 which exceeds the single-byte limit.

Values below 0x64 (100) worked because their decimal representation
happens to be valid hex under 256.

Fix by detecting integer strings (no colons, known size) and parsing
as base-0 to handle both decimal and hex input correctly.

Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1824
Signed-off-by: Brian Witte <brianwitte@mailfence.com>
---
 src/datatype.c                                | 17 ++++++++
 .../testcases/nft-f/0033ether_addr_hex_0      | 42 +++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100755 tests/shell/testcases/nft-f/0033ether_addr_hex_0

diff --git a/src/datatype.c b/src/datatype.c
index 18973851..d7817ef2 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -581,6 +581,23 @@ static struct error_record *lladdr_type_parse(struct parse_ctx *ctx,
 	const char *s = sym->identifier;
 	unsigned int len, n;
 
+	if (sym->dtype->size && !strchr(s, ':')) {
+		unsigned int size = sym->dtype->size / BITS_PER_BYTE;
+		uint64_t val;
+
+		errno = 0;
+		val = strtoull(s, &p, 0);
+		if (!errno && !*p && !(val >> (size * 8))) {
+			for (n = 0; n < size; n++)
+				buf[size - n - 1] = (val >> (n * 8)) & 0xff;
+
+			*res = constant_expr_alloc(&sym->location, sym->dtype,
+						   BYTEORDER_BIG_ENDIAN,
+						   sym->dtype->size, buf);
+			return NULL;
+		}
+	}
+
 	for (len = 0;;) {
 		n = strtoul(s, &p, 16);
 		if (s == p || n > 0xff)
diff --git a/tests/shell/testcases/nft-f/0033ether_addr_hex_0 b/tests/shell/testcases/nft-f/0033ether_addr_hex_0
new file mode 100755
index 00000000..149b8f52
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0033ether_addr_hex_0
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table inet t {
+	chain c {
+		ether daddr 0x0 accept
+		ether daddr 0x64 accept
+		ether daddr 0xff accept
+		ether daddr 0x100 accept
+		ether daddr 0xffffffffffff accept
+		ether daddr aa accept
+		ether saddr de:ad:be:ef:00:01 accept
+	}
+}"
+
+EXPECTED="table inet t {
+	chain c {
+		ether daddr 00:00:00:00:00:00 accept
+		ether daddr 00:00:00:00:00:64 accept
+		ether daddr 00:00:00:00:00:ff accept
+		ether daddr 00:00:00:00:01:00 accept
+		ether daddr ff:ff:ff:ff:ff:ff accept
+		ether daddr 00:00:00:00:00:aa accept
+		ether saddr de:ad:be:ef:00:01 accept
+	}
+}"
+
+$NFT -f - <<< "$RULESET"
+
+GET="$($NFT list ruleset)"
+
+if [ "$EXPECTED" != "$GET" ] ; then
+	$DIFF -u <(echo "$EXPECTED") <(echo "$GET")
+	exit 1
+fi
+
+# overflow must fail
+if $NFT add rule inet t c ether daddr 0x1000000000000 accept 2>/dev/null; then
+	echo "FAIL: overflow not rejected" >&2
+	exit 1
+fi
-- 
2.47.3

Re: [nft PATCH] datatype: fix ether address parsing of integer values

[Florian Westphal]

Brian Witte <brianwitte@mailfence.com> wrote:
> When parsing "ether daddr 0x64", integer_expr converts the hex value
> to decimal string "100". lladdr_type_parse then interprets this as
> hex, yielding 0x100 = 256 which exceeds the single-byte limit.

Yes, but it looks like it breaks backwards compatibility.

'ether daddr 99' is parsed as 00:00:00:00:00:99.
After your patch, its parsed as 00:00:00:00:00:63.

So I'm not sure we can fix this.

Re: [nft PATCH] datatype: fix ether address parsing of integer values

[Brian Witte]

Florian Westphal <fw@strlen.de> wrote:
> Yes, but it looks like it breaks backwards compatibility.
>
> 'ether daddr 99' is parsed as 00:00:00:00:00:99.
> After your patch, its parsed as 00:00:00:00:00:63.
>
> So I'm not sure we can fix this.

You're right, I hadn't considered that case. Thanks for the review.

Brian