diff --git a/docs/changelog/108089.yaml b/docs/changelog/108089.yaml new file mode 100644 index 000000000000..02fb6349185a --- /dev/null +++ b/docs/changelog/108089.yaml @@ -0,0 +1,6 @@ +pr: 108089 +summary: "ES|QL: limit query depth to 500 levels" +area: ES|QL +type: bug +issues: + - 107752 diff --git a/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java b/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java index 6b8b9a33e028..38f8ad4766b7 100644 --- a/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java +++ b/test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java @@ -263,7 +263,7 @@ public class HeapAttackIT extends ESRestTestCase { columns = columns.item(matchesMap().entry("name", "c").entry("type", "long")); columns = columns.item(matchesMap().entry("name", "d").entry("type", "long")); columns = columns.item(matchesMap().entry("name", "e").entry("type", "long")); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < 20; i++) { columns = columns.item(matchesMap().entry("name", "i0" + i).entry("type", "long")); } assertMap(map, matchesMap().entry("columns", columns).entry("values", hasSize(10_000))); @@ -272,7 +272,7 @@ public class HeapAttackIT extends ESRestTestCase { @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/108104") public void testTooManyEval() throws IOException { initManyLongs(); - assertCircuitBreaks(() -> manyEval(1000)); + assertCircuitBreaks(() -> manyEval(490)); } private Response manyEval(int evalLines) throws IOException { @@ -280,7 +280,7 @@ public class HeapAttackIT extends ESRestTestCase { query.append("FROM manylongs"); for (int e = 0; e < evalLines; e++) { query.append("\n| EVAL "); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < 20; i++) { if (i != 0) { query.append(", "); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java index c7342325764d..7ab3dfefc325 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java @@ -87,6 +87,13 @@ import static org.elasticsearch.xpack.ql.util.StringUtils.WILDCARD; public abstract class ExpressionBuilder extends IdentifierBuilder { + private int expressionDepth = 0; + + /** + * Maximum depth for nested expressions + */ + public static final int MAX_EXPRESSION_DEPTH = 500; + private final Map params; ExpressionBuilder(Map params) { @@ -94,7 +101,19 @@ public abstract class ExpressionBuilder extends IdentifierBuilder { } protected Expression expression(ParseTree ctx) { - return typedParsing(this, ctx, Expression.class); + expressionDepth++; + if (expressionDepth > MAX_EXPRESSION_DEPTH) { + throw new ParsingException( + "ESQL statement exceeded the maximum expression depth allowed ({}): [{}]", + MAX_EXPRESSION_DEPTH, + ctx.getParent().getText() + ); + } + try { + return typedParsing(this, ctx, Expression.class); + } finally { + expressionDepth--; + } } protected List expressions(List contexts) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java index 8906014adeec..aea835c11ad3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java @@ -76,6 +76,13 @@ import static org.elasticsearch.xpack.ql.parser.ParserUtils.visitList; public class LogicalPlanBuilder extends ExpressionBuilder { + private int queryDepth = 0; + + /** + * Maximum number of commands allowed per query + */ + public static final int MAX_QUERY_DEPTH = 500; + public LogicalPlanBuilder(Map params) { super(params); } @@ -95,9 +102,21 @@ public class LogicalPlanBuilder extends ExpressionBuilder { @Override public LogicalPlan visitCompositeQuery(EsqlBaseParser.CompositeQueryContext ctx) { - LogicalPlan input = plan(ctx.query()); - PlanFactory makePlan = typedParsing(this, ctx.processingCommand(), PlanFactory.class); - return makePlan.apply(input); + queryDepth++; + if (queryDepth > MAX_QUERY_DEPTH) { + throw new ParsingException( + "ESQL statement exceeded the maximum query depth allowed ({}): [{}]", + MAX_QUERY_DEPTH, + ctx.getText() + ); + } + try { + LogicalPlan input = plan(ctx.query()); + PlanFactory makePlan = typedParsing(this, ctx.processingCommand(), PlanFactory.class); + return makePlan.apply(input); + } finally { + queryDepth--; + } } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java index 82334afbffd0..b2a074ab4723 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java @@ -48,6 +48,7 @@ import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Gre import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.esql.parser.EsqlParser; +import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.EsRelation; @@ -122,6 +123,8 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; import static org.elasticsearch.xpack.esql.EsqlTestUtils.statsForMissingField; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; import static org.elasticsearch.xpack.esql.SerializationTestUtils.assertSerialization; +import static org.elasticsearch.xpack.esql.parser.ExpressionBuilder.MAX_EXPRESSION_DEPTH; +import static org.elasticsearch.xpack.esql.parser.LogicalPlanBuilder.MAX_QUERY_DEPTH; import static org.elasticsearch.xpack.esql.plan.physical.AggregateExec.Mode.FINAL; import static org.elasticsearch.xpack.esql.plan.physical.AggregateExec.Mode.PARTIAL; import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.CARTESIAN_POINT; @@ -4023,6 +4026,118 @@ public class PhysicalPlanOptimizerTests extends ESTestCase { ); } + public void testMaxExpressionDepth_cast() { + StringBuilder queryBuilder = new StringBuilder(randomBoolean() ? "row a = 1" : "row a = 1 | eval b = a"); + queryBuilder.append("::long::int".repeat(MAX_EXPRESSION_DEPTH / 2 - 1)); + var query = queryBuilder.toString(); + + physicalPlan(query); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(query + "::long")); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + } + + public void testMaxExpressionDepth_math() { + StringBuilder queryBuilder = new StringBuilder(randomBoolean() ? "row a = 1" : "row a = 1 | eval b = a"); + String expression = " " + randomFrom("+", "-", "*", "/") + " 1"; + queryBuilder.append(expression.repeat(MAX_EXPRESSION_DEPTH - 2)); + var query = queryBuilder.toString(); + + physicalPlan(query); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(query + expression)); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + } + + public void testMaxExpressionDepth_boolean() { + StringBuilder queryBuilder = new StringBuilder(randomBoolean() ? "row a = true " : "row a = true | eval b = a"); + String expression = " " + randomFrom("and", "or") + " true"; + queryBuilder.append(expression.repeat(MAX_EXPRESSION_DEPTH - 2)); + var query = queryBuilder.toString(); + + physicalPlan(query); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(query + expression)); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + } + + public void testMaxExpressionDepth_parentheses() { + String query = "row a = true | eval b = "; + StringBuilder expression = new StringBuilder("(".repeat(MAX_EXPRESSION_DEPTH / 2 - 1)); + expression.append("a"); + expression.append(")".repeat(MAX_EXPRESSION_DEPTH / 2 - 1)); + + physicalPlan(query + expression); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(query + "(" + expression + ")")); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + } + + public void testMaxExpressionDepth_mixed() { + String prefix = "abs("; + String suffix = " + 12)"; + + String from = "row a = 1 | eval b = "; + + StringBuilder queryBuilder = new StringBuilder(); + queryBuilder.append(prefix.repeat(MAX_EXPRESSION_DEPTH / 2 - 1)); + queryBuilder.append("a"); + queryBuilder.append(suffix.repeat(MAX_EXPRESSION_DEPTH / 2 - 1)); + var expression = queryBuilder.toString(); + + physicalPlan(from + expression); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(from + prefix + expression + suffix)); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + } + + public void testMaxQueryDepth() { + StringBuilder from = new StringBuilder("row a = 1 "); + for (int i = 0; i < MAX_QUERY_DEPTH; i++) { + from.append(randomBoolean() ? "| where a > 0 " : " | eval b" + i + " = a + " + i); + } + physicalPlan(from.toString()); + var e = expectThrows(ParsingException.class, () -> physicalPlan(from + (randomBoolean() ? "| sort a" : " | eval c = 10"))); + assertThat(e.getMessage(), containsString("ESQL statement exceeded the maximum query depth allowed (" + MAX_QUERY_DEPTH + ")")); + } + + public void testMaxQueryDepthPlusExpressionDepth() { + StringBuilder mainQuery = new StringBuilder("row a = 1 "); + for (int i = 0; i < MAX_QUERY_DEPTH; i++) { + mainQuery.append(" | eval b" + i + " = a + " + i); + } + + physicalPlan(mainQuery.toString()); + + var cast = "::long::int".repeat(MAX_EXPRESSION_DEPTH / 2 - 2) + "::long"; + + physicalPlan(mainQuery + cast); + + var e = expectThrows(ParsingException.class, () -> physicalPlan(mainQuery + cast + "::int")); + assertThat( + e.getMessage(), + containsString("ESQL statement exceeded the maximum expression depth allowed (" + MAX_EXPRESSION_DEPTH + ")") + ); + + e = expectThrows(ParsingException.class, () -> physicalPlan(mainQuery + cast + " | eval x = 10")); + assertThat(e.getMessage(), containsString("ESQL statement exceeded the maximum query depth allowed (" + MAX_QUERY_DEPTH + ")")); + } + @SuppressWarnings("SameParameterValue") private static void assertFilterCondition( Filter filter,