[PATCH nft] json: reject too long interface names

[PATCH nft] json: reject too long interface names

[Florian Westphal]

Blamed commit added a length check on ifnames to the bison parser.
Unfortunately that wasn't enough, json parser has the same issue.

Bogon results in:
BUG: Interface length 44 exceeds limit
nft: src/mnl.c:742: nft_dev_add: Assertion `0' failed.

After patch, included bogon results in:
Error: Invalid device at index 0. name d2345678999999999999999999999999999999012345 too long

I intentionally did not extend evaluate.c to catch this, past sentiment
was that frontends should not send garbage.

I'll send a followup patch to also catch this from eval stage in case there
are further reports for frontends passing in such long names.

Fixes: fa52bc225806 ("parser: reject zero-length interface names")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_json.c                             | 17 +++-
 .../nft-j-f/dev_name_parser_overflow_crash    | 90 +++++++++++++++++++
 2 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-j-f/dev_name_parser_overflow_crash

diff --git a/src/parser_json.c b/src/parser_json.c
index e3dd14cda350..3195d529cbbc 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2951,7 +2951,13 @@ static struct expr *json_parse_devs(struct json_ctx *ctx, json_t *root)
 	size_t index;
 
 	if (!json_unpack(root, "s", &dev)) {
-		tmp = constant_expr_alloc(int_loc, &string_type,
+		if (strlen(dev) >= IFNAMSIZ) {
+			json_error(ctx, "Device name %s too long", dev);
+			expr_free(expr);
+			return NULL;
+		}
+
+		tmp = constant_expr_alloc(int_loc, &ifname_type,
 					  BYTEORDER_HOST_ENDIAN,
 					  strlen(dev) * BITS_PER_BYTE, dev);
 		compound_expr_add(expr, tmp);
@@ -2969,7 +2975,14 @@ static struct expr *json_parse_devs(struct json_ctx *ctx, json_t *root)
 			expr_free(expr);
 			return NULL;
 		}
-		tmp = constant_expr_alloc(int_loc, &string_type,
+
+		if (strlen(dev) >= IFNAMSIZ) {
+			json_error(ctx, "Device name %s too long at index %zu", dev, index);
+			expr_free(expr);
+			return NULL;
+		}
+
+		tmp = constant_expr_alloc(int_loc, &ifname_type,
 					  BYTEORDER_HOST_ENDIAN,
 					  strlen(dev) * BITS_PER_BYTE, dev);
 		compound_expr_add(expr, tmp);
diff --git a/tests/shell/testcases/bogons/nft-j-f/dev_name_parser_overflow_crash b/tests/shell/testcases/bogons/nft-j-f/dev_name_parser_overflow_crash
new file mode 100644
index 000000000000..8303c5c1cad9
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-j-f/dev_name_parser_overflow_crash
@@ -0,0 +1,90 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "netdev",
+        "name": "filter1",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "netdev",
+        "table": "filter1",
+        "name": "Main_Ingress1",
+        "handle": 0,
+        "dev": "lo",
+        "type": "filter",
+        "hook": "ingress",
+        "prio": -500,
+        "policy": "accept"
+      }
+    },
+    {
+      "table": {
+        "family": "netdev",
+        "name": "filter2",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "netdev",
+        "table": "filter2",
+        "name": "Main_Ingress2",
+        "handle": 0,
+        "dev": [
+          "d2345678999999999999999999999999999999012345",
+          "lo"
+        ],
+        "type": "filter",
+        "hook": "ingress",
+        "prio": -500,
+        "policy": "accept"
+      }
+    },
+    {
+      "table": {
+        "family": "netdev",
+        "name": "filter3",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "netdev",
+        "table": "filter3",
+        "name": "Main_Ingress3",
+        "handle": 0,
+        "dev": [
+          "d23456789012345",
+          "lo"
+        ],
+        "type": "filter",
+        "hook": "ingress",
+        "prio": -500,
+        "policy": "accept"
+      }
+    },
+    {
+      "chain": {
+        "family": "netdev",
+        "table": "filter3",
+        "name": "Main_Egress3",
+        "handle": 0,
+        "dev": "lo",
+        "type": "filter",
+        "hook": "egress",
+        "prio": -500,
+        "policy": "accept"
+      }
+    }
+  ]
+}
-- 
2.49.0

Re: [PATCH nft] json: reject too long interface names

[Pablo Neira Ayuso]

On Tue, Jun 24, 2025 at 11:46:59PM +0200, Florian Westphal wrote:
> Blamed commit added a length check on ifnames to the bison parser.
> Unfortunately that wasn't enough, json parser has the same issue.
> 
> Bogon results in:
> BUG: Interface length 44 exceeds limit
> nft: src/mnl.c:742: nft_dev_add: Assertion `0' failed.
> 
> After patch, included bogon results in:
> Error: Invalid device at index 0. name d2345678999999999999999999999999999999012345 too long
> 
> I intentionally did not extend evaluate.c to catch this, past sentiment
> was that frontends should not send garbage.
> 
> I'll send a followup patch to also catch this from eval stage in case there
> are further reports for frontends passing in such long names.
> 
> Fixes: fa52bc225806 ("parser: reject zero-length interface names")
> Signed-off-by: Florian Westphal <fw@strlen.de>

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