EQL: Disallow chained comparisons (#62567)

Expressions like `1 = 2 = 3 = 4` or `1 < 2 = 3 >= 4` were treated with leftmost priority: ((1 = 2) = 3) = 4 which can lead to confusing results. Since such expressions don't make so much change for EQL filters we disallow them in the parser to prevent unexpected results from their bad usage. Major DBs like PostgreSQL and Oracle also disallow them in their SQL syntax. (counter example would be MySQL which interprets them as we did before with leftmost priority). Fixes: #61654
elastic · Sep 18, 2020 · 8f94981 · 8f94981
1 parent 95c1488
commit 8f94981
Show file tree

Hide file tree

Showing 8 changed files with 494 additions and 353 deletions.
diff --git a/x-pack/plugin/eql/src/main/antlr/EqlBase.g4 b/x-pack/plugin/eql/src/main/antlr/EqlBase.g4
@@ -84,11 +84,15 @@ booleanExpression
 
 
 valueExpression
-    : primaryExpression predicate?                                                      #valueExpressionDefault
-    | operator=(MINUS | PLUS) valueExpression                                           #arithmeticUnary
-    | left=valueExpression operator=(ASTERISK | SLASH | PERCENT) right=valueExpression  #arithmeticBinary
-    | left=valueExpression operator=(PLUS | MINUS) right=valueExpression                #arithmeticBinary
-    | left=valueExpression comparisonOperator right=valueExpression                     #comparison
+    : operatorExpression                                                                      #valueExpressionDefault
+    | left=operatorExpression comparisonOperator right=operatorExpression                     #comparison
+    ;
+
+operatorExpression
+    : primaryExpression predicate?                                                            #operatorExpressionDefault
+    | operator=(MINUS | PLUS) operatorExpression                                              #arithmeticUnary
+    | left=operatorExpression operator=(ASTERISK | SLASH | PERCENT) right=operatorExpression  #arithmeticBinary
+    | left=operatorExpression operator=(PLUS | MINUS) right=operatorExpression                #arithmeticBinary
     ;
 
 // workaround for

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseBaseListener.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseBaseListener.java
@@ -263,6 +263,18 @@ class EqlBaseBaseListener implements EqlBaseListener {
    * <p>The default implementation does nothing.</p>
    */
   @Override public void exitComparison(EqlBaseParser.ComparisonContext ctx) { }
+  /**
+   * {@inheritDoc}
+   *
+   * <p>The default implementation does nothing.</p>
+   */
+  @Override public void enterOperatorExpressionDefault(EqlBaseParser.OperatorExpressionDefaultContext ctx) { }
+  /**
+   * {@inheritDoc}
+   *
+   * <p>The default implementation does nothing.</p>
+   */
+  @Override public void exitOperatorExpressionDefault(EqlBaseParser.OperatorExpressionDefaultContext ctx) { }
   /**
    * {@inheritDoc}
    *

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseBaseVisitor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseBaseVisitor.java
@@ -158,6 +158,13 @@ class EqlBaseBaseVisitor<T> extends AbstractParseTreeVisitor<T> implements EqlBa
    * {@link #visitChildren} on {@code ctx}.</p>
    */
   @Override public T visitComparison(EqlBaseParser.ComparisonContext ctx) { return visitChildren(ctx); }
+  /**
+   * {@inheritDoc}
+   *
+   * <p>The default implementation returns the result of calling
+   * {@link #visitChildren} on {@code ctx}.</p>
+   */
+  @Override public T visitOperatorExpressionDefault(EqlBaseParser.OperatorExpressionDefaultContext ctx) { return visitChildren(ctx); }
   /**
    * {@inheritDoc}
    *

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseListener.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseListener.java
@@ -229,27 +229,39 @@ interface EqlBaseListener extends ParseTreeListener {
    * @param ctx the parse tree
    */
   void exitComparison(EqlBaseParser.ComparisonContext ctx);
+  /**
+   * Enter a parse tree produced by the {@code operatorExpressionDefault}
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
+   * @param ctx the parse tree
+   */
+  void enterOperatorExpressionDefault(EqlBaseParser.OperatorExpressionDefaultContext ctx);
+  /**
+   * Exit a parse tree produced by the {@code operatorExpressionDefault}
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
+   * @param ctx the parse tree
+   */
+  void exitOperatorExpressionDefault(EqlBaseParser.OperatorExpressionDefaultContext ctx);
   /**
    * Enter a parse tree produced by the {@code arithmeticBinary}
-   * labeled alternative in {@link EqlBaseParser#valueExpression}.
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
    * @param ctx the parse tree
    */
   void enterArithmeticBinary(EqlBaseParser.ArithmeticBinaryContext ctx);
   /**
    * Exit a parse tree produced by the {@code arithmeticBinary}
-   * labeled alternative in {@link EqlBaseParser#valueExpression}.
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
    * @param ctx the parse tree
    */
   void exitArithmeticBinary(EqlBaseParser.ArithmeticBinaryContext ctx);
   /**
    * Enter a parse tree produced by the {@code arithmeticUnary}
-   * labeled alternative in {@link EqlBaseParser#valueExpression}.
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
    * @param ctx the parse tree
    */
   void enterArithmeticUnary(EqlBaseParser.ArithmeticUnaryContext ctx);
   /**
    * Exit a parse tree produced by the {@code arithmeticUnary}
-   * labeled alternative in {@link EqlBaseParser#valueExpression}.
+   * labeled alternative in {@link EqlBaseParser#operatorExpression}.
    * @param ctx the parse tree
    */
   void exitArithmeticUnary(EqlBaseParser.ArithmeticUnaryContext ctx);