ES|QL: limit query depth to 500 levels (#108089)

This commit is contained in:
Luigi Dell'Aquila 2024-05-02 17:42:41 +02:00 committed by GitHub
parent 6d20cef931
commit b277d5b033
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 166 additions and 7 deletions

View file

@ -0,0 +1,6 @@
pr: 108089
summary: "ES|QL: limit query depth to 500 levels"
area: ES|QL
type: bug
issues:
- 107752

View file

@ -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", "c").entry("type", "long"));
columns = columns.item(matchesMap().entry("name", "d").entry("type", "long")); columns = columns.item(matchesMap().entry("name", "d").entry("type", "long"));
columns = columns.item(matchesMap().entry("name", "e").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")); columns = columns.item(matchesMap().entry("name", "i0" + i).entry("type", "long"));
} }
assertMap(map, matchesMap().entry("columns", columns).entry("values", hasSize(10_000))); 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") @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/108104")
public void testTooManyEval() throws IOException { public void testTooManyEval() throws IOException {
initManyLongs(); initManyLongs();
assertCircuitBreaks(() -> manyEval(1000)); assertCircuitBreaks(() -> manyEval(490));
} }
private Response manyEval(int evalLines) throws IOException { private Response manyEval(int evalLines) throws IOException {
@ -280,7 +280,7 @@ public class HeapAttackIT extends ESRestTestCase {
query.append("FROM manylongs"); query.append("FROM manylongs");
for (int e = 0; e < evalLines; e++) { for (int e = 0; e < evalLines; e++) {
query.append("\n| EVAL "); query.append("\n| EVAL ");
for (int i = 0; i < 10; i++) { for (int i = 0; i < 20; i++) {
if (i != 0) { if (i != 0) {
query.append(", "); query.append(", ");
} }

View file

@ -87,6 +87,13 @@ import static org.elasticsearch.xpack.ql.util.StringUtils.WILDCARD;
public abstract class ExpressionBuilder extends IdentifierBuilder { 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<Token, TypedParamValue> params; private final Map<Token, TypedParamValue> params;
ExpressionBuilder(Map<Token, TypedParamValue> params) { ExpressionBuilder(Map<Token, TypedParamValue> params) {
@ -94,7 +101,19 @@ public abstract class ExpressionBuilder extends IdentifierBuilder {
} }
protected Expression expression(ParseTree ctx) { protected Expression expression(ParseTree ctx) {
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); return typedParsing(this, ctx, Expression.class);
} finally {
expressionDepth--;
}
} }
protected List<Expression> expressions(List<? extends ParserRuleContext> contexts) { protected List<Expression> expressions(List<? extends ParserRuleContext> contexts) {

View file

@ -76,6 +76,13 @@ import static org.elasticsearch.xpack.ql.parser.ParserUtils.visitList;
public class LogicalPlanBuilder extends ExpressionBuilder { 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<Token, TypedParamValue> params) { public LogicalPlanBuilder(Map<Token, TypedParamValue> params) {
super(params); super(params);
} }
@ -95,9 +102,21 @@ public class LogicalPlanBuilder extends ExpressionBuilder {
@Override @Override
public LogicalPlan visitCompositeQuery(EsqlBaseParser.CompositeQueryContext ctx) { public LogicalPlan visitCompositeQuery(EsqlBaseParser.CompositeQueryContext ctx) {
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()); LogicalPlan input = plan(ctx.query());
PlanFactory makePlan = typedParsing(this, ctx.processingCommand(), PlanFactory.class); PlanFactory makePlan = typedParsing(this, ctx.processingCommand(), PlanFactory.class);
return makePlan.apply(input); return makePlan.apply(input);
} finally {
queryDepth--;
}
} }
@Override @Override

View file

@ -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.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.parser.EsqlParser; 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.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.Enrich;
import org.elasticsearch.xpack.esql.plan.logical.EsRelation; 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.statsForMissingField;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
import static org.elasticsearch.xpack.esql.SerializationTestUtils.assertSerialization; 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.FINAL;
import static org.elasticsearch.xpack.esql.plan.physical.AggregateExec.Mode.PARTIAL; import static org.elasticsearch.xpack.esql.plan.physical.AggregateExec.Mode.PARTIAL;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.CARTESIAN_POINT; 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") @SuppressWarnings("SameParameterValue")
private static void assertFilterCondition( private static void assertFilterCondition(
Filter filter, Filter filter,