This is an automated email from the ASF dual-hosted git repository. bdelacretaz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
The following commit(s) were added to refs/heads/master by this push: new 08fe036 SLING-12198 - make maxQueryTokens and maxWhitespaceTokens configurable (#39) 08fe036 is described below commit 08fe0362b60a24a67c8ef84f0c3d05dd80b4ebc5 Author: Lenard Palko (Lenny) <lenard.pa...@gmail.com> AuthorDate: Fri Dec 22 17:43:51 2023 +0100 SLING-12198 - make maxQueryTokens and maxWhitespaceTokens configurable (#39) --- .../graphql/core/engine/DefaultQueryExecutor.java | 35 ++++++ .../graphql/core/engine/MaxQueryTokensTest.java | 129 +++++++++++++++++++++ .../graphql/core/engine/ResourceQueryTestBase.java | 6 +- 3 files changed, 169 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java index 0c092f1..aa5a975 100644 --- a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java +++ b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java @@ -27,9 +27,12 @@ import java.util.Optional; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.Consumer; import javax.script.ScriptException; +import graphql.GraphQLContext; +import graphql.parser.ParserOptions; import org.apache.sling.api.resource.Resource; import org.apache.sling.graphql.api.SchemaProvider; import org.apache.sling.graphql.api.SlingDataFetcher; @@ -114,6 +117,10 @@ public class DefaultQueryExecutor implements QueryExecutor { private final Lock writeLock = readWriteLock.writeLock(); private final SchemaGenerator schemaGenerator = new SchemaGenerator(); + private int maxQueryTokens; + + private int maxWhitespaceTokens; + @Reference private RankedSchemaProviders schemaProvider; @@ -136,6 +143,20 @@ public class DefaultQueryExecutor implements QueryExecutor { " cached and reused, rather than parsed by the engine all the time. The cache is a LRU and will store up to this number of schemas." ) int schemaCacheSize() default 128; + + @AttributeDefinition( + name = "Max Query Tokens", + description = "The number of GraphQL query tokens to parse. This is a safety measure to avoid denial of service attacks." + + " Change ONLY if you know exactly what you are doing." + ) + int maxQueryTokens() default 15000; + + @AttributeDefinition( + name = "Max Whitespace Tokens", + description = "The number of GraphQL query whitespace tokens to parse. This is a safety measure to avoid denial of service attacks." + + " Change ONLY if you know exactly what you are doing." + ) + int maxWhitespaceTokens() default 200000; } private class ExecutionContext { @@ -155,9 +176,20 @@ public class DefaultQueryExecutor implements QueryExecutor { input = ExecutionInput.newExecutionInput() .query(query) .variables(variables) + .graphQLContext(getGraphQLContextBuilder()) .build(); } + + private Consumer<GraphQLContext.Builder> getGraphQLContextBuilder() { + final ParserOptions parserOptions = ParserOptions.getDefaultParserOptions().transform(builder -> + builder + .maxTokens(maxQueryTokens) + .maxWhitespaceTokens(maxWhitespaceTokens) + .build() + ); + return builder -> builder.put(ParserOptions.class, parserOptions); + } } @Activate @@ -166,6 +198,9 @@ public class DefaultQueryExecutor implements QueryExecutor { if (schemaCacheSize < 0) { schemaCacheSize = 0; } + maxQueryTokens = config.maxQueryTokens(); + maxWhitespaceTokens = config.maxWhitespaceTokens(); + resourceToHashMap = new LRUCache<>(schemaCacheSize); hashToSchemaMap = new LRUCache<>(schemaCacheSize); } diff --git a/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java b/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java new file mode 100644 index 0000000..0c10ce0 --- /dev/null +++ b/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java @@ -0,0 +1,129 @@ +/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ +package org.apache.sling.graphql.core.engine; + +import org.apache.sling.graphql.core.mocks.CharacterTypeResolver; +import org.apache.sling.graphql.core.mocks.EchoDataFetcher; +import org.apache.sling.graphql.core.mocks.TestUtil; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath; +import static org.hamcrest.Matchers.equalTo; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; + +/** Test the SLING-12198 configurable max tokens */ +@RunWith(Parameterized.class) +public class MaxQueryTokensTest extends ResourceQueryTestBase { + private static final int DEFAULT_MAX_QUERY_TOKENS = 15000; + private static final String INVALID = "invalid "; + private static final String WHITESPACE = " {\t"; + private final Integer maxQueryTokens; + private final Integer maxWhitespaceTokens; + private final int nOk; + private final int nTokensFailure; + private final int nWhitespaceFailure; + + @Parameters(name = "{0}") + public static Collection<Object[]> data() { + final List<Object[]> result = new ArrayList<>(); + result.add(new Object[] { "default values", null, null, DEFAULT_MAX_QUERY_TOKENS - 1, DEFAULT_MAX_QUERY_TOKENS, -1 }); + result.add(new Object[] { "same values", 100, 100, 99, 100, 100 }); + result.add(new Object[] { "more whitespace", 100, 200, 99, 100, 150 }); + return result; + } + + public MaxQueryTokensTest(String name, Integer maxTokens, Integer maxWhitespaceTokens, int nOk, int nTokensFailure, + int nWhitespaceFailure) { + this.maxQueryTokens = maxTokens; + this.maxWhitespaceTokens = maxWhitespaceTokens; + this.nOk = nOk; + this.nTokensFailure = nTokensFailure; + this.nWhitespaceFailure = nWhitespaceFailure; + } + + protected Map<String, Object> getQueryExecutorProperties() { + final Map<String, Object> props = new HashMap<>(); + if (maxQueryTokens != null) { + props.put("maxQueryTokens", maxQueryTokens); + } + if (maxWhitespaceTokens != null) { + props.put("maxWhitespaceTokens", maxWhitespaceTokens); + } + return props; + } + + static String repeat(String what, int howMany) { + final StringBuffer sb = new StringBuffer(); + for (int i = 0; i < howMany; i++) { + sb.append(what); + } + return sb.toString(); + } + + private void assertQueryFailure(String query, String expectedError) throws Exception { + final String json = queryJSON(query); + assertThat(json, hasJsonPath("$.errors[0].extensions.classification", is("InvalidSyntax"))); + assertThat(json, hasJsonPath("$.errors[0].message", containsString(expectedError))); + } + + protected void setupAdditionalServices() { + TestUtil.registerSlingTypeResolver(context.bundleContext(), "character/resolver", new CharacterTypeResolver()); + TestUtil.registerSlingDataFetcher(context.bundleContext(), "echoNS/echo", new EchoDataFetcher(null)); + } + + @Test + public void verifyQueriesWork() throws Exception { + final String json = queryJSON("{ currentResource { path resourceType } }"); + assertThat(json, hasJsonPath("$.data.currentResource")); + assertThat(json, hasJsonPath("$.data.currentResource.path", equalTo(resource.getPath()))); + assertThat(json, hasJsonPath("$.data.currentResource.resourceType", equalTo(resource.getResourceType()))); + } + + @Test + public void numberOfTokensOk() throws Exception { + assertQueryFailure(repeat(INVALID, nOk), "Invalid syntax with offending token"); + } + + @Test + public void tooManyTokens() throws Exception { + assertQueryFailure(repeat(INVALID, nTokensFailure), + "'grammar' tokens have been presented. To prevent Denial Of Service"); + } + + @Test + public void tooManyWhitespaceTokens() throws Exception { + if (nWhitespaceFailure >= 0) { + assertQueryFailure(repeat(WHITESPACE, nWhitespaceFailure), + "'whitespace' tokens have been presented. To prevent Denial Of Service"); + } + } +} \ No newline at end of file diff --git a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java index 0482e19..34c92fc 100644 --- a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java +++ b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java @@ -70,7 +70,11 @@ public abstract class ResourceQueryTestBase { context.registerInjectActivateService(new SlingTypeResolverSelector()); context.registerInjectActivateService(new SlingScalarsProvider()); context.registerInjectActivateService(new RankedSchemaProviders()); - context.registerInjectActivateService(new DefaultQueryExecutor()); + context.registerInjectActivateService(new DefaultQueryExecutor(), getQueryExecutorProperties()); + } + + protected Map<String, Object> getQueryExecutorProperties() { + return null; } protected String queryJSON(String stmt) throws Exception {