On Mon, Mar 31, 2025 at 06:15:30PM +0200, Florian Westphal wrote:
> Florian Westphal <fw@xxxxxxxxx> wrote:
> > The bison parser doesn't allow this to happen due to grammar
> > restrictions, but the json input has no such issues.
> >
> > The bogon input assigns 'notrack' which triggers:
> > BUG: unknown stateful statement type 19
> > nft: src/netlink_linearize.c:1061: netlink_gen_stmt_stateful: Assertion `0' failed.
> >
> > After patch, we get:
> > Error: map statement must be stateful
>
> On the same subject of 'do I fix this in evaluate.c or parser_json.c':
>
> cat bla
> table t {
> set sc {
> type inet_service . ifname
> }
>
> chain c {
> tcp dport . bla* @sc accept
> }
> }
> nft -f bla
> BUG: unknown expression type prefix
> nft: src/netlink_linearize.c:914: netlink_gen_expr: Assertion `0' failed.
> Aborted (core dumped)
>
> I can either fix this in evaluate.c, or I try to rework both
> parser_bison.y and parser_json.c to no longer allow prefix expressions
> when specifying the lookup key.
It is probably too complex from the parser in this case.
> I suspect that fixing it in evaluate.c is going to be a lot simpler.
I agree.
But for things that are obviously incorrect at parsing stage and that
are simple to reject, like the empty string case for goto/jump in
verdict map, then it is easy to reject early. The json parser already
rejects unexisting/unsupported keys, the parser is already making a
first pass in validating the input.
I don't think it is a matter of picking between validating _only_ from
parser or from evaluation, I think tightening from parser (when
trivial) and evaluation for more complex invalid input is fine.
> We can't disable prefixes in concatenations, its valid for set element
> keys, but I suspect that we can use recursion counter to figure out if
> the concatenation is on the RHS of something else (such as part of
> EXPR_SET_ELEM).
>
> I'll work on it tomorrow.
Thanks!
