diff options
| author | Florian Westphal <fw@strlen.de> | 2025-06-24 23:46:59 +0200 |
|---|---|---|
| committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2025-08-13 20:45:46 +0200 |
| commit | 0cca60e27813cb84f7c8b13cd78d33060e708a0a (patch) | |
| tree | 8384a62e075f4bf06a56f8b1626586dd814f2785 | |
| parent | 9f0e2e1373d13f3a7d8615367c0ab25be3296a94 (diff) | |
json: reject too long interface names
commit bed99830c4c63eae205c28a7ff914737bedb199d upstream.
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>
| -rw-r--r-- | src/parser_json.c | 17 | ||||
| -rw-r--r-- | tests/shell/testcases/bogons/nft-j-f/dev_name_parser_overflow_crash | 90 |
2 files changed, 105 insertions, 2 deletions
diff --git a/src/parser_json.c b/src/parser_json.c index a4481aed..b213478e 100644 --- a/src/parser_json.c +++ b/src/parser_json.c @@ -3006,7 +3006,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); @@ -3024,7 +3030,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 00000000..8303c5c1 --- /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" + } + } + ] +} |