10731.diff

commit f54c5e5986cf8192c8741d5f482ab5d11ec55455
Author: Ben Pfaff <blp@ovn.org>
Date:   Thu Oct 11 12:44:33 2018 -0700

    expr: Disallow < <= >= > comparisons against empty value set.
    
    OVN expression syntax does not allow a literal empty value set, like {}.
    Rather, any literal value set has to have at least one value.  However,
    value sets that originate from address sets or from port groups can be
    empty.  In such a case, == and != comparisons are allowed but < <= >= >
    should be errors.  The actual implementation failed to properly disallow
    the latter and instead tried to access the first element of the value set,
    a bad read.  This fixes the problem.
    
    Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
    Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767
    Signed-off-by: Ben Pfaff <blp@ovn.org>
    Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index b50475245..8cfdf34fa 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -560,68 +560,73 @@ static struct expr *
 make_cmp(struct expr_context *ctx,
          const struct expr_field *f, enum expr_relop r,
          struct expr_constant_set *cs)
 {
     struct expr *e = NULL;
 
     if (!type_check(ctx, f, cs)) {
         goto exit;
     }
 
     if (r != EXPR_R_EQ && r != EXPR_R_NE) {
         if (cs->in_curlies) {
             lexer_error(ctx->lexer, "Only == and != operators may be used "
                         "with value sets.");
             goto exit;
         }
         if (f->symbol->level == EXPR_L_NOMINAL ||
             f->symbol->level == EXPR_L_BOOLEAN) {
             lexer_error(ctx->lexer, "Only == and != operators may be used "
                         "with %s field %s.",
                         expr_level_to_string(f->symbol->level),
                         f->symbol->name);
             goto exit;
         }
+        if (!cs->n_values) {
+            lexer_error(ctx->lexer, "Only == and != operators may be used "
+                        "to compare a field against an empty value set.");
+            goto exit;
+        }
         if (cs->values[0].masked) {
             lexer_error(ctx->lexer, "Only == and != operators may be used "
                         "with masked constants.  Consider using subfields "
                         "instead (e.g. eth.src[0..15] > 0x1111 in place of "
                         "eth.src > 00:00:00:00:11:11/00:00:00:00:ff:ff).");
             goto exit;
         }
     }
 
     if (f->symbol->level == EXPR_L_NOMINAL) {
         if (f->symbol->predicate) {
             ovs_assert(f->symbol->width > 0);
             for (size_t i = 0; i < cs->n_values; i++) {
                 const union mf_subvalue *value = &cs->values[i].value;
                 bool positive = (value->integer & htonll(1)) != 0;
                 positive ^= r == EXPR_R_NE;
                 positive ^= ctx->not;
                 if (!positive) {
                     const char *name = f->symbol->name;
                     lexer_error(ctx->lexer,
                                 "Nominal predicate %s may only be tested "
                                 "positively, e.g. `%s' or `%s == 1' but not "
                                 "`!%s' or `%s == 0'.",
                                 name, name, name, name, name);
                     goto exit;
                 }
             }
         } else if (r != (ctx->not ? EXPR_R_NE : EXPR_R_EQ)) {
             lexer_error(ctx->lexer, "Nominal field %s may only be tested for "
                         "equality (taking enclosing `!' operators into "
                         "account).", f->symbol->name);
             goto exit;
         }
     }
 
     if (!cs->n_values) {
         e = expr_create_boolean(r == EXPR_R_NE);
         goto exit;
     }
     e = make_cmp__(f, r, &cs->values[0]);
     for (size_t i = 1; i < cs->n_values; i++) {
         e = expr_combine(r == EXPR_R_EQ ? EXPR_T_OR : EXPR_T_AND,
                          e, make_cmp__(f, r, &cs->values[i]));
     }
diff --git a/tests/ovn.at b/tests/ovn.at
index 886981d51..71518d97d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -355,6 +355,8 @@ ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' expect
 eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' expecting constant.
 
 ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((())))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) => Parentheses nested too deeply.
+
+ct_label > $set4 => Only == and != operators may be used to compare a field against an empty value set.
 ]])
 sed 's/ =>.*//' test-cases.txt > input.txt
 sed 's/.* => //' test-cases.txt > expout