src: do not merge a set with a erroneous one

The included sample causes a crash because we attempt to
range-merge a prefix expression with a symbolic expression.

The first set is evaluated, the symbol expression evaluation fails
and nft queues an error message ("Could not resolve hostname").

However, nft continues evaluation.

nft then encounters the same set definition again and merges the
new content with the preceeding one.

But the first set structure is dodgy, it still contains the
unresolved symbolic expression.

That then makes nft crash (assert) in the set internals.

There are various different incarnations of this issue, but the low
level set processing code does not allow for any partially transformed
expressions to still remain.

Before:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
BUG: invalid range expression type binop
nft: src/expression.c:1479: range_expr_value_low: Assertion `0' failed.

After:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
invalid_range_expr_type_binop:4:18-25: Error: Could not resolve hostname: Name or service not known
elements = { 1&.141.0.1 - 192.168.0.2}
             ^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/rule.h b/include/rule.h
index 56a9495d46b0ec04e39486f89586c5ae0d67505f..3a833cf3a458831ecefb24776418907b35f100a0 100644 (file)
--- a/include/rule.h
+++ b/include/rule.h
@@ -329,6 +329,7 @@ void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
  * @policy:    set mechanism policy
  * @automerge: merge adjacents and overlapping elements, if possible
  * @comment:   comment
+ * @errors:    expr evaluation errors seen
  * @desc.size:         count of set elements
  * @desc.field_len:    length of single concatenated fields, bytes
  * @desc.field_count:  count of concatenated fields
@@ -353,6 +354,7 @@ struct set {
        bool                    root;
        bool                    automerge;
        bool                    key_typeof_valid;
+       bool                    errors;
        const char              *comment;
        struct {
                uint32_t        size;

diff --git a/src/evaluate.c b/src/evaluate.c
index 8196d3d314dcab3a73cd0492b8e2400e84b336dc..5b585714180d08067fab3243d94fc6470fb9be74 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4845,8 +4845,10 @@ static int elems_evaluate(struct eval_ctx *ctx, struct set *set)
 
                __expr_set_context(&ctx->ectx, set->key->dtype,
                                   set->key->byteorder, set->key->len, 0);
-               if (expr_evaluate(ctx, &set->init) < 0)
+               if (expr_evaluate(ctx, &set->init) < 0) {
+                       set->errors = true;
                        return -1;
+               }
                if (set->init->etype != EXPR_SET)
                        return expr_error(ctx->msgs, set->init, "Set %s: Unexpected initial type %s, missing { }?",
                                          set->handle.set.name, expr_name(set->init));

diff --git a/src/intervals.c b/src/intervals.c
index 5a88a8eb20bd7515397c689427b21b149195ad3b..68728349e999c6875caa3213c7126410e415e0c2 100644 (file)
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -132,7 +132,7 @@ static void set_sort_splice(struct expr *init, struct set *set)
        set_to_range(init);
        list_expr_sort(&init->expressions);
 
-       if (!existing_set)
+       if (!existing_set || existing_set->errors)
                return;
 
        if (existing_set->init) {

diff --git a/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop b/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
new file mode 100644 (file)
index 0000000..514d6ff
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
@@ -0,0 +1,12 @@
+table ip x {
+       map z {
+               type ipv4_addr : ipv4_addr
+               elements = { 1&.141.0.1 - 192.168.0.2}
+       }
+
+       map z {
+               type ipv4_addr : ipv4_addr
+               flags interval
+               elements = { 10.141.0.0, * : 192.168.0.4 }
+       }
+}