| Message ID | 146296620273.3706.17267671338035433056.stgit@nfdev2.cica.es |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, May 11, 2016 at 01:30:02PM +0200, Arturo Borrero Gonzalez wrote: > If we are evaluating an EXPR_SET_REF, check if right->dtype is not NULL. > We can hit SEGFAULT if for whatever reason the referenced object does not > exists. > > Using this testfile (note the invalid set syntax): > > % cat test.nft > flush ruleset > add table t > add chain t c > add set t s {type ipv4_addr\;} > add rule t c ip saddr @s > > Without this patch: > > % nft -f test.nft > Segmentation fault > > With this patch: > > % nft -f test.nft > t.nft:4:28-28: Error: syntax error, unexpected junk, expecting newline or semicolon > add set t s {type ipv4_addr\;} > ^ > t.nft:4:13-29: Error: set definition does not specify key data type > add set t s {type ipv4_addr\;} > ^^^^^^^^^^^^^^^^^ > t.nft:5:23-24: Error: the referenced object does not exists I have reworded this to: "the referenced set does not exist" > add rule t c ip saddr @s > ~~~~~~~~ ^^ Applied, thanks Arturo. > Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> > --- > src/evaluate.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/src/evaluate.c b/src/evaluate.c > index 7444d09..6840790 100644 > --- a/src/evaluate.c > +++ b/src/evaluate.c > @@ -1210,16 +1210,33 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) > > switch (rel->op) { > case OP_LOOKUP: > - /* A literal set expression implicitly declares the set */ > - if (right->ops->type == EXPR_SET) > + switch (right->ops->type) { > + case EXPR_SET: > + /* A literal set expression implicitly declares > + * the set > + */ > right = rel->right = > - implicit_set_declaration(ctx, left->dtype, left->len, right); > - else if (!datatype_equal(left->dtype, right->dtype)) > - return expr_binary_error(ctx->msgs, right, left, > - "datatype mismatch, expected %s, " > - "set has type %s", > - left->dtype->desc, > - right->dtype->desc); > + implicit_set_declaration(ctx, left->dtype, > + left->len, right); > + break; > + case EXPR_SET_REF: > + if (right->dtype == NULL) > + return expr_binary_error(ctx->msgs, right, > + left, "the referenced" > + " object does not " > + "exists"); > + if (!datatype_equal(left->dtype, right->dtype)) > + return expr_binary_error(ctx->msgs, right, > + left, "datatype " > + "mismatch, expected " > + "%s, set has type %s", > + left->dtype->desc, > + right->dtype->desc); > + break; > + default: > + BUG("unhandled right expression type %u\n", > + right->ops->type); I have also replaced this by the typical: BUG("Unknown expression %s\n", right->ops->name); -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13 May 2016 at 11:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Wed, May 11, 2016 at 01:30:02PM +0200, Arturo Borrero Gonzalez wrote: >> If we are evaluating an EXPR_SET_REF, check if right->dtype is not NULL. >> We can hit SEGFAULT if for whatever reason the referenced object does not >> exists. >> >> Using this testfile (note the invalid set syntax): >> >> % cat test.nft >> flush ruleset >> add table t >> add chain t c >> add set t s {type ipv4_addr\;} >> add rule t c ip saddr @s >> >> Without this patch: >> >> % nft -f test.nft >> Segmentation fault >> >> With this patch: >> >> % nft -f test.nft >> t.nft:4:28-28: Error: syntax error, unexpected junk, expecting newline or semicolon >> add set t s {type ipv4_addr\;} >> ^ >> t.nft:4:13-29: Error: set definition does not specify key data type >> add set t s {type ipv4_addr\;} >> ^^^^^^^^^^^^^^^^^ >> t.nft:5:23-24: Error: the referenced object does not exists > > I have reworded this to: "the referenced set does not exist" > Ok, I used the generic word 'object' because this could apply to maps as well. >> add rule t c ip saddr @s >> ~~~~~~~~ ^^ > > Applied, thanks Arturo. thanks
diff --git a/src/evaluate.c b/src/evaluate.c index 7444d09..6840790 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -1210,16 +1210,33 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) switch (rel->op) { case OP_LOOKUP: - /* A literal set expression implicitly declares the set */ - if (right->ops->type == EXPR_SET) + switch (right->ops->type) { + case EXPR_SET: + /* A literal set expression implicitly declares + * the set + */ right = rel->right = - implicit_set_declaration(ctx, left->dtype, left->len, right); - else if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "set has type %s", - left->dtype->desc, - right->dtype->desc); + implicit_set_declaration(ctx, left->dtype, + left->len, right); + break; + case EXPR_SET_REF: + if (right->dtype == NULL) + return expr_binary_error(ctx->msgs, right, + left, "the referenced" + " object does not " + "exists"); + if (!datatype_equal(left->dtype, right->dtype)) + return expr_binary_error(ctx->msgs, right, + left, "datatype " + "mismatch, expected " + "%s, set has type %s", + left->dtype->desc, + right->dtype->desc); + break; + default: + BUG("unhandled right expression type %u\n", + right->ops->type); + } /* Data for range lookups needs to be in big endian order */ if (right->set->flags & SET_F_INTERVAL &&
If we are evaluating an EXPR_SET_REF, check if right->dtype is not NULL. We can hit SEGFAULT if for whatever reason the referenced object does not exists. Using this testfile (note the invalid set syntax): % cat test.nft flush ruleset add table t add chain t c add set t s {type ipv4_addr\;} add rule t c ip saddr @s Without this patch: % nft -f test.nft Segmentation fault With this patch: % nft -f test.nft t.nft:4:28-28: Error: syntax error, unexpected junk, expecting newline or semicolon add set t s {type ipv4_addr\;} ^ t.nft:4:13-29: Error: set definition does not specify key data type add set t s {type ipv4_addr\;} ^^^^^^^^^^^^^^^^^ t.nft:5:23-24: Error: the referenced object does not exists add rule t c ip saddr @s ~~~~~~~~ ^^ Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> --- src/evaluate.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html