* [PATCH nft] parser_json: only allow concatenations with 2 or more expressions
@ 2025-04-02 5:18 Florian Westphal
2025-04-02 8:44 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Florian Westphal @ 2025-04-02 5:18 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
The bison grammar enforces this implicitly by grammar rules, e.g.
"mark . ip saddr" or similar, i.e., ALL concatenation expressions
consist of at least two elements.
But this doesn't apply to the json frontend which just uses an
array: it can be empty or only contain one element.
The included reproducer makes the eval stage set the "concatenation" flag
on the interval set. This prevents the needed conversion code to turn the
element values into ranges from getting run.
The reproducer asserts with:
nft: src/intervals.c:786: setelem_to_interval: Assertion `key->etype == EXPR_RANGE_VALUE' failed.
Convert the assertion to BUG() so we can see what element type got passed
to the set interval code in case we have further issues in this area.
Reject 0-or-1-element concatenations from the json parser.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/intervals.c | 4 ++-
src/parser_json.c | 20 ++++++++------
.../set_with_single_value_concat_assert | 26 +++++++++++++++++++
3 files changed, 41 insertions(+), 9 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
diff --git a/src/intervals.c b/src/intervals.c
index 71ad210bf759..1ab443bcde87 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -783,7 +783,9 @@ int setelem_to_interval(const struct set *set, struct expr *elem,
next_key = NULL;
}
- assert(key->etype == EXPR_RANGE_VALUE);
+ if (key->etype != EXPR_RANGE_VALUE)
+ BUG("key must be RANGE_VALUE, not %s\n", expr_name(key));
+
assert(!next_key || next_key->etype == EXPR_RANGE_VALUE);
/* skip end element for adjacents intervals in anonymous sets. */
diff --git a/src/parser_json.c b/src/parser_json.c
index 94d09212314f..724cba881623 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1251,6 +1251,16 @@ static struct expr *json_parse_binop_expr(struct json_ctx *ctx,
return binop_expr_alloc(int_loc, thisop, left, right);
}
+static struct expr *json_check_concat_expr(struct json_ctx *ctx, struct expr *e)
+{
+ if (e->size >= 2)
+ return e;
+
+ json_error(ctx, "Concatenation with %d elements is illegal", e->size);
+ expr_free(e);
+ return NULL;
+}
+
static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
const char *type, json_t *root)
{
@@ -1284,7 +1294,7 @@ static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
}
compound_expr_add(expr, tmp);
}
- return expr;
+ return expr ? json_check_concat_expr(ctx, expr) : NULL;
}
static struct expr *json_parse_prefix_expr(struct json_ctx *ctx,
@@ -1748,13 +1758,7 @@ static struct expr *json_parse_dtype_expr(struct json_ctx *ctx, json_t *root)
compound_expr_add(expr, i);
}
- if (list_empty(&expr->expressions)) {
- json_error(ctx, "Empty concatenation");
- expr_free(expr);
- return NULL;
- }
-
- return expr;
+ return json_check_concat_expr(ctx, expr);
} else if (json_is_object(root)) {
const char *key;
json_t *val;
diff --git a/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert b/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
new file mode 100644
index 000000000000..c99a26683470
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
@@ -0,0 +1,26 @@
+{
+ "nftables": [
+ {
+ "metainfo": {
+ "version": "nftables", "json_schema_version": 1
+ }
+ },
+ {
+ "table": {
+ "family": "ip",
+ "name": "t",
+ "handle": 0
+ }
+ },
+ {
+ "set": {
+ "family": "ip",
+ "name": "s",
+ "table": "t",
+ "type": [ "ifname" ],
+ "flags": [ "interval" ],
+ "elem": [ [] ]
+ }
+ }
+ ]
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH nft] parser_json: only allow concatenations with 2 or more expressions
2025-04-02 5:18 [PATCH nft] parser_json: only allow concatenations with 2 or more expressions Florian Westphal
@ 2025-04-02 8:44 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2025-04-02 8:44 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Wed, Apr 02, 2025 at 07:18:18AM +0200, Florian Westphal wrote:
> The bison grammar enforces this implicitly by grammar rules, e.g.
> "mark . ip saddr" or similar, i.e., ALL concatenation expressions
> consist of at least two elements.
>
> But this doesn't apply to the json frontend which just uses an
> array: it can be empty or only contain one element.
>
> The included reproducer makes the eval stage set the "concatenation" flag
> on the interval set. This prevents the needed conversion code to turn the
> element values into ranges from getting run.
>
> The reproducer asserts with:
> nft: src/intervals.c:786: setelem_to_interval: Assertion `key->etype == EXPR_RANGE_VALUE' failed.
>
> Convert the assertion to BUG() so we can see what element type got passed
> to the set interval code in case we have further issues in this area.
>
> Reject 0-or-1-element concatenations from the json parser.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thanks
> ---
> src/intervals.c | 4 ++-
> src/parser_json.c | 20 ++++++++------
> .../set_with_single_value_concat_assert | 26 +++++++++++++++++++
> 3 files changed, 41 insertions(+), 9 deletions(-)
> create mode 100644 tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
>
> diff --git a/src/intervals.c b/src/intervals.c
> index 71ad210bf759..1ab443bcde87 100644
> --- a/src/intervals.c
> +++ b/src/intervals.c
> @@ -783,7 +783,9 @@ int setelem_to_interval(const struct set *set, struct expr *elem,
> next_key = NULL;
> }
>
> - assert(key->etype == EXPR_RANGE_VALUE);
> + if (key->etype != EXPR_RANGE_VALUE)
> + BUG("key must be RANGE_VALUE, not %s\n", expr_name(key));
> +
> assert(!next_key || next_key->etype == EXPR_RANGE_VALUE);
>
> /* skip end element for adjacents intervals in anonymous sets. */
> diff --git a/src/parser_json.c b/src/parser_json.c
> index 94d09212314f..724cba881623 100644
> --- a/src/parser_json.c
> +++ b/src/parser_json.c
> @@ -1251,6 +1251,16 @@ static struct expr *json_parse_binop_expr(struct json_ctx *ctx,
> return binop_expr_alloc(int_loc, thisop, left, right);
> }
>
> +static struct expr *json_check_concat_expr(struct json_ctx *ctx, struct expr *e)
> +{
> + if (e->size >= 2)
> + return e;
> +
> + json_error(ctx, "Concatenation with %d elements is illegal", e->size);
> + expr_free(e);
> + return NULL;
> +}
> +
> static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
> const char *type, json_t *root)
> {
> @@ -1284,7 +1294,7 @@ static struct expr *json_parse_concat_expr(struct json_ctx *ctx,
> }
> compound_expr_add(expr, tmp);
> }
> - return expr;
> + return expr ? json_check_concat_expr(ctx, expr) : NULL;
> }
>
> static struct expr *json_parse_prefix_expr(struct json_ctx *ctx,
> @@ -1748,13 +1758,7 @@ static struct expr *json_parse_dtype_expr(struct json_ctx *ctx, json_t *root)
> compound_expr_add(expr, i);
> }
>
> - if (list_empty(&expr->expressions)) {
> - json_error(ctx, "Empty concatenation");
> - expr_free(expr);
> - return NULL;
> - }
> -
> - return expr;
> + return json_check_concat_expr(ctx, expr);
> } else if (json_is_object(root)) {
> const char *key;
> json_t *val;
> diff --git a/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert b/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
> new file mode 100644
> index 000000000000..c99a26683470
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-j-f/set_with_single_value_concat_assert
> @@ -0,0 +1,26 @@
> +{
> + "nftables": [
> + {
> + "metainfo": {
> + "version": "nftables", "json_schema_version": 1
> + }
> + },
> + {
> + "table": {
> + "family": "ip",
> + "name": "t",
> + "handle": 0
> + }
> + },
> + {
> + "set": {
> + "family": "ip",
> + "name": "s",
> + "table": "t",
> + "type": [ "ifname" ],
> + "flags": [ "interval" ],
> + "elem": [ [] ]
> + }
> + }
> + ]
> +}
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-02 8:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-02 5:18 [PATCH nft] parser_json: only allow concatenations with 2 or more expressions Florian Westphal
2025-04-02 8:44 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.