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@xxxxxxxxx>
Reviewed-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
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
>
>
