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 <[email protected]>
> ---
>  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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to